770 lines
26 KiB
Markdown
770 lines
26 KiB
Markdown
# zeling_v2 代码全面评审
|
||
|
||
> 评审日期：2026-05-11
|
||
> 评审范围：`Assets/Scripts/` 全部模块（约 25 个 Assembly Definition，覆盖 Combat、Player、Enemies、Core、UI、World 等）
|
||
> 评审标准：以《空洞骑士》《Celeste》《Neon Abyss》等成熟商业 2D 动作游戏代码质量为参照基准
|
||
> 说明：本次评审在上一轮优化（AdvancedCodeReview.md）的基础上进行，反映所有已实施改动后的当前代码状态。
|
||
|
||
---
|
||
|
||
## 目录
|
||
|
||
1. [评分总览](#1-评分总览)
|
||
2. [架构设计](#2-架构设计)
|
||
3. [性能](#3-性能)
|
||
4. [可扩展性](#4-可扩展性)
|
||
5. [编辑器友好性](#5-编辑器友好性)
|
||
6. [使用便利性（DX）](#6-使用便利性dx)
|
||
7. [问题优先级汇总表](#7-问题优先级汇总表)
|
||
8. [优化建议](#8-优化建议)
|
||
|
||
---
|
||
|
||
## 1. 评分总览
|
||
|
||
| 维度 | 得分（满分 10） | 与商业基准差距 |
|
||
|---|---|---|
|
||
| **架构设计** | 7.5 | 基础扎实，单例滥用是主要拖分项 |
|
||
| **性能** | 7.0 | 热路径已优化，若干 Update 开销尚存 |
|
||
| **可扩展性** | 7.5 | Assembly 划分优秀，配置驱动待完善 |
|
||
| **编辑器友好性** | 6.5 | 工具链有亮点，SO 验证未落地 |
|
||
| **使用便利性** | 7.0 | API 设计清晰，异步一致性有缺口 |
|
||
| **综合** | **7.1** | 优于大多数独立游戏，接近中等商业质量 |
|
||
|
||
---
|
||
|
||
## 2. 架构设计
|
||
|
||
### 2.1 优点
|
||
|
||
#### ✅ 事件频道（SO Event Channel）模式落地质量高
|
||
|
||
`BaseEventChannelSO<T>` 泛型事件基类设计完善：
|
||
|
||
```csharp
|
||
// BaseEventChannelSO.cs
|
||
public EventSubscription Subscribe(Action<T> callback)
|
||
{
|
||
OnEventRaised += callback;
|
||
return new EventSubscription(() => OnEventRaised -= callback);
|
||
}
|
||
```
|
||
|
||
- 返回 `EventSubscription` 可组合到 `CompositeDisposable`，避免手动取消订阅遗漏
|
||
- `EventBusMonitor`（仅 Editor）记录 256 条事件日志，帧号+监听器数量一应俱全
|
||
- 约 35+ 个具体频道类型覆盖全模块，系统间全部解耦
|
||
|
||
> 商业对比：与《GDC 2017 - Unite Austin》推荐的 SO 架构一致，执行完整度超过大多数独立参考项目。
|
||
|
||
#### ✅ 25 个 Assembly Definition 依赖拓扑明确
|
||
|
||
```
|
||
BaseGames.Core.Events
|
||
↑
|
||
BaseGames.Core
|
||
↑
|
||
BaseGames.Combat BaseGames.Parry BaseGames.Player
|
||
↑ ↑
|
||
BaseGames.Enemies BaseGames.Player.States
|
||
```
|
||
|
||
- `BaseGames.Parry` 不依赖 `BaseGames.Combat`，`HurtBox.ConsumeParry()` 无跨程序集数据，干净
|
||
- `BaseGames.Player.States` 单独隔离，16 个状态类不污染 Player 程序集
|
||
|
||
#### ✅ 玩家 FSM：POCO 状态类 + 类型字典派发
|
||
|
||
```csharp
|
||
// PlayerController.cs
|
||
private readonly Dictionary<Type, PlayerStateBase> _states = new();
|
||
// ...
|
||
_states[typeof(AttackState)] = new AttackState(this);
|
||
public T GetState<T>() where T : PlayerStateBase
|
||
=> _states.TryGetValue(typeof(T), out var s) ? (T)s : null;
|
||
```
|
||
|
||
- 状态不继承 `MonoBehaviour`，无 Unity 序列化负担
|
||
- `GetState<T>()` 零 GC，O(1) 字典查找
|
||
- 状态基类 `PlayerStateBase` 提供 `Owner`/`Input`/`Anim`/`Cfg` 便捷属性，子类代码清晰
|
||
|
||
#### ✅ ServiceLocator 实现规范
|
||
|
||
```csharp
|
||
public static TInterface GetOrDefault<TInterface>(TInterface fallback = default) { ... }
|
||
#if UNITY_EDITOR
|
||
public static void OverrideForTest<TInterface>(TInterface mock) { ... }
|
||
public static void Reset() { ... }
|
||
#endif
|
||
```
|
||
|
||
- 接口类型注册，支持依赖倒置
|
||
- 提供 Editor 专用测试 override/reset，测试友好
|
||
|
||
#### ✅ 对象池完整实现
|
||
|
||
- Addressables 异步预热（`WarmupAsync` / `WarmupCoroutine` 双版本）
|
||
- `LinkedList<PooledObject>` 活跃列表，LRU O(1) 强制回收
|
||
- `GetComponentCached<T>()` 扩展方法避免重复反射
|
||
|
||
#### ✅ SaveManager HMAC 校验 + 版本迁移
|
||
|
||
```csharp
|
||
// SaveMigrator.Migrate(loaded) 确保旧存档平滑升级
|
||
loaded = SaveMigrator.Migrate(loaded);
|
||
```
|
||
|
||
- `RunFireAndForget` 正确捕获 async void 中的异常
|
||
- `Formatting.None` 减少序列化体积
|
||
|
||
---
|
||
|
||
### 2.2 问题
|
||
|
||
#### ⚠️ P1：单例（static Instance）与 ServiceLocator 混用
|
||
|
||
项目同时使用两种全局访问模式：
|
||
|
||
| 类 | 访问方式 |
|
||
|---|---|
|
||
| `GameManager` | `static Instance` |
|
||
| `SaveManager` | `static Instance` |
|
||
| `MapManager` | `static Instance` |
|
||
| `QuestManager` | `static Instance` |
|
||
| `GlobalObjectPool` | `static Instance` |
|
||
| `IPlatformService` | `ServiceLocator.Get<>()` |
|
||
| `ISceneService` | `ServiceLocator.Get<>()` |
|
||
| `IAudioService` | `ServiceLocator.Get<>()` |
|
||
|
||
混用导致：
|
||
- `MapManager.Instance?.Register(this)` 在 `OnEnable` 中调用，若 MapManager 未激活则静默失败
|
||
- 无法对 `SaveManager`、`QuestManager` 进行单元测试
|
||
- 多场景重新加载时 Instance 生命周期难以追踪
|
||
|
||
**商业标准：** 《Hollow Knight》等成熟项目统一通过 DI 容器或 ServiceLocator 管理；绝不混用。
|
||
|
||
#### ⚠️ P1：EnemyBase 仍保留 Awake 中 FindWithTag（已修复，但注释混乱）
|
||
|
||
修复前 Awake 仍调用 `FindWithTag`，已在本次评审中同步修复（添加事件订阅，移除旧代码）。说明之前的优化不完整，留下了"Phase 1 注释"与"Phase 2 实现"共存的混乱状态：
|
||
|
||
```csharp
|
||
// Start() 的兜底查找与 Awake() 的 FindWithTag 同时存在（已修复）
|
||
// 这种 "Phase 注释" 模式在多次迭代后容易造成死代码残留
|
||
```
|
||
|
||
**建议：** 移除所有 `// Phase 1：` / `// Phase 2：` 标注，改用 `// TODO:` 或 git branch 管理迭代。
|
||
|
||
#### ⚠️ P2：EnemyBase.ForceState() 是空实现
|
||
|
||
```csharp
|
||
public void ForceState(EnemyStateType newState)
|
||
{
|
||
_currentState = newState;
|
||
// Phase 2：根据状态播放对应动画 / 触发硬直计时
|
||
}
|
||
```
|
||
|
||
只更新枚举值，无任何动画/计时副作用。`TakeDamage()` 调用 `ForceState(Hurt)` 后敌人没有实际硬直反馈，是一个静默的逻辑空洞。
|
||
|
||
#### ⚠️ P2：UIManager 忽略商店 ID 参数
|
||
|
||
```csharp
|
||
private void OpenShop(string _) => OpenPanel(_shopRoot);
|
||
```
|
||
|
||
`_onShopOpen` 携带 `string shopId`（商店 ID），但 `UIManager` 直接丢弃。若游戏后期出现多个商店，此处需要重构，届时影响面较大。
|
||
|
||
#### ⚠️ P2：GameStateMachine 与 GameStateId 的 struct 滥用
|
||
|
||
`GameStateId` 被设计为 struct（值类型）但内部包含 string `Id`：
|
||
|
||
```csharp
|
||
// 示例推断：GameStateId 包含 string Id 字段
|
||
if (state == GameStates.Gameplay || state == GameStates.BossFight)
|
||
```
|
||
|
||
struct 值比较需要 `IEquatable<T>` 重写，否则 `==` 走装箱比较，每次 `HandleGameStateChanged` 触发都可能有 GC 分配。
|
||
|
||
---
|
||
|
||
## 3. 性能
|
||
|
||
### 3.1 优点
|
||
|
||
#### ✅ 热路径 GetComponent 全部缓存
|
||
|
||
```csharp
|
||
// HurtBox.cs - Awake 缓存，每次受击无 GetComponent
|
||
private IStatusEffectable _statusEffectable;
|
||
private void Awake() => _statusEffectable = GetComponentInParent<IStatusEffectable>();
|
||
```
|
||
|
||
#### ✅ 距离计算使用 sqrMagnitude
|
||
|
||
```csharp
|
||
// EnemyBase.Update()
|
||
_stats.SqrDistanceToPlayer = ((Vector2)_playerTransform.position - (Vector2)transform.position).sqrMagnitude;
|
||
// IsPlayerInRange
|
||
return _stats.SqrDistanceToPlayer <= range * range;
|
||
```
|
||
|
||
避免每帧 `Mathf.Sqrt`，100 个敌人同屏时节省约 100 次浮点开平方。
|
||
|
||
#### ✅ 对象池 LRU O(1) 回收
|
||
|
||
`LinkedList<PooledObject>` 头节点始终是最早 Spawn 的对象，强制回收 O(1)，替换原 `List.RemoveAt(0)` 的 O(n) 移位。
|
||
|
||
#### ✅ 事件频道无 GC 分配（C# event）
|
||
|
||
`Action<T>` delegate 调用无堆分配，优于 `UnityEvent`（有对象封装）。
|
||
|
||
#### ✅ StatusEffectManager 双结构避免重复遍历
|
||
|
||
```csharp
|
||
private readonly List<StatusEffect> _activeList = new(); // Update 遍历
|
||
private readonly Dictionary<StatusEffectType, StatusEffect> _activeIndex = new(); // O(1) 类型查找
|
||
```
|
||
|
||
逆序遍历 List 安全移除，`Dictionary` 保证施加同类效果时 O(1) 查找堆叠。
|
||
|
||
---
|
||
|
||
### 3.2 问题
|
||
|
||
#### ⚠️ P1：PlayerMovement.UpdateFacing() 使用 localScale 翻转
|
||
|
||
```csharp
|
||
transform.localScale = new Vector3(Mathf.Abs(s.x) * dir, s.y, s.z);
|
||
```
|
||
|
||
`localScale` 负值翻转会：
|
||
1. 影响子节点所有 Collider 的物理中心（尤其是圆形 Collider）
|
||
2. 导致 UI Canvas World Space 子元素同步翻转
|
||
3. 与粒子系统、Shader（uv 方向）产生兼容问题
|
||
|
||
**商业标准：** Celeste / HK 均使用 `SpriteRenderer.flipX = true` 或旋转 `transform.rotation`，绝不翻转 Scale。
|
||
|
||
#### ⚠️ P1：GameServiceRegistrar 使用 FindObjectsOfType（场景加载时触发）
|
||
|
||
```csharp
|
||
// GameServiceRegistrar.EnsureSingleAudioListener()
|
||
AudioListener[] listeners = FindObjectsOfType<AudioListener>(true);
|
||
```
|
||
|
||
在 `SceneManager.sceneLoaded` 回调中调用 `FindObjectsOfType`，每次场景加载全场景扫描。若场景中 GameObject 数量大，这是一个可测量的加载卡顿点。
|
||
|
||
**建议：** 改为每个场景相机自行注册 AudioListener，或在场景 Bootstrap 时主动禁用多余监听器。
|
||
|
||
#### ⚠️ P2：DashState 使用魔法数字兜底
|
||
|
||
```csharp
|
||
float force = _config != null ? _config.JumpForce : 18f; // PlayerMovement
|
||
Stats?.BeginInvincibility(Cfg != null ? Cfg.DashDuration + 0.05f : 0.23f); // DashState
|
||
Move?.SetGravityScale(Cfg != null ? Cfg.DefaultGravityScale : 3f);
|
||
```
|
||
|
||
每个状态类都有 `Cfg != null ? ... : hardcoded` 二元表达式，分散了数值配置源，调试时不知道实际值来自哪里。
|
||
|
||
**建议：** 在 `PlayerStateBase` 基类或 `PlayerController` 中加 `[RequireComponent]` 或 `Awake` 断言，确保配置 SO 永远不为 null，消除运行时兜底逻辑。
|
||
|
||
#### ⚠️ P2：多处 Coroutine + async Task 并存（GlobalObjectPool）
|
||
|
||
```csharp
|
||
public IEnumerator WarmupCoroutine() { ... } // 相同逻辑
|
||
public async Task WarmupAsync() { ... } // 相同逻辑
|
||
```
|
||
|
||
同一预热逻辑实现了两遍，维护时需同步修改两个版本。
|
||
|
||
**建议：** 统一为 `async Task`，在需要 Coroutine 调用方处用 `StartCoroutine(WarmupAsync().AsCoroutine())` 桥接。
|
||
|
||
#### ⚠️ P3：LocalizationManager.Get() 静默吞掉异常
|
||
|
||
```csharp
|
||
catch
|
||
{
|
||
return entryKey;
|
||
}
|
||
```
|
||
|
||
本地化读取失败时返回 Key 字符串并且不打 Log，生产环境难以排查本地化数据缺失问题。
|
||
|
||
---
|
||
|
||
## 4. 可扩展性
|
||
|
||
### 4.1 优点
|
||
|
||
#### ✅ 配置数据与逻辑完全分离
|
||
|
||
- `PlayerAnimationConfigSO`：动画片段 + HitBox 时间点 + 硬直时长
|
||
- `PlayerMovementConfigSO`：加速度、跳跃力、土狼时间、冲刺参数
|
||
- `EnemyStatsSO`：HP、攻击力、视野、寻路参数
|
||
|
||
设计师可在不修改代码的情况下独立调整全部数值。
|
||
|
||
#### ✅ 攻击连击段数动态读取
|
||
|
||
```csharp
|
||
int maxCombo = AnimCfg?.GroundAttacks?.Length ?? 1;
|
||
if (_comboIndex < maxCombo - 1) { ... }
|
||
```
|
||
|
||
`GroundAttacks[]` 数组长度决定连击段数，添加新连击只需在 SO 中添加一个 AnimationClip。
|
||
|
||
#### ✅ IPathAgent 接口抽象导航层
|
||
|
||
```csharp
|
||
protected IPathAgent _nav; // EnemyBase
|
||
// 由子类/Inspector注入，或 GetComponent<IPathAgent>()
|
||
```
|
||
|
||
PathBerserker2d / A* Pathfinding / NavMesh2D 均可通过实现 IPathAgent 无缝替换。
|
||
|
||
#### ✅ ISaveable / SaveMigrator 版本化存档
|
||
|
||
```csharp
|
||
loaded = SaveMigrator.Migrate(loaded);
|
||
```
|
||
|
||
版本迁移管道已建立，存档格式变更时可平滑升级旧存档。
|
||
|
||
---
|
||
|
||
### 4.2 问题
|
||
|
||
#### ⚠️ P1：EnemyBase.ForceState() 无扩展点
|
||
|
||
敌人状态机是扁平的 `EnemyStateType` 枚举 + switch（或 if/else），没有对应玩家 FSM 的"POCO 状态类 + 字典"设计。添加新敌人状态需要：
|
||
|
||
1. 扩展 `EnemyStateType` 枚举
|
||
2. 在 `ForceState()` / `Update()` 等多处手动添加分支
|
||
|
||
**与玩家 FSM 差距明显。** 建议将敌人也迁移到 POCO 状态类或完全依赖 Behavior Designer 行为树，二选一，避免两套系统共存。
|
||
|
||
#### ⚠️ P1：WorldStateRegistry 没有泛化的持久化标记 API
|
||
|
||
```csharp
|
||
public bool IsCollected(string id) => _collectedIds.Contains(id);
|
||
public bool IsDoorOpened(string id) => _openedDoors.Contains(id);
|
||
public bool IsDestroyed(string id) => _destroyedObjects.Contains(id);
|
||
```
|
||
|
||
每增加一类世界实体状态，都需要：1）添加新的 `HashSet<string>` 字段；2）添加两个方法；3）更新 `LoadFromSave`/`GetAllFlags`。可扩展性差。
|
||
|
||
**建议：** 统一为 `Dictionary<string, HashSet<string>>` 按"类别"键隔离，或引入 `enum WorldObjectCategory` 参数化访问。
|
||
|
||
#### ⚠️ P2：技能系统（SkillManager）冷却计时硬编码三个槽
|
||
|
||
```csharp
|
||
private float _soulCooldown;
|
||
private float _spirit1Cooldown;
|
||
private float _spirit2Cooldown;
|
||
```
|
||
|
||
支持的技能槽数量在编译时固定为 3。若后续要求更多技能槽，需要修改 SkillManager 代码。
|
||
|
||
**建议：** 改用 `Dictionary<FormSkillSO, float>` 动态冷却表，支持任意数量技能。
|
||
|
||
---
|
||
|
||
## 5. 编辑器友好性
|
||
|
||
### 5.1 优点
|
||
|
||
#### ✅ EventBusMonitor：运行时事件日志
|
||
|
||
```csharp
|
||
#if UNITY_EDITOR
|
||
EventBusMonitor.Record(name, value?.ToString(), listenerCount, Time.frameCount);
|
||
#endif
|
||
```
|
||
|
||
Editor 模式下所有 SO 事件的触发都会被记录（频道名、负载、监听器数量、帧号），通过自定义窗口可以实时追踪事件流。这是商业游戏才有的调试基础设施。
|
||
|
||
#### ✅ WorldMarker.OnDrawGizmosSelected()
|
||
|
||
```csharp
|
||
Gizmos.color = _markerType switch
|
||
{
|
||
WorldMarkerType.Objective => Color.yellow,
|
||
WorldMarkerType.NPC => Color.cyan,
|
||
WorldMarkerType.PointOfInterest => Color.green,
|
||
...
|
||
};
|
||
```
|
||
|
||
场景中直接可视化标记类型，减少 playtest 调试成本。
|
||
|
||
#### ✅ IValidatable 接口
|
||
|
||
```csharp
|
||
public interface IValidatable
|
||
{
|
||
IEnumerable<string> Validate();
|
||
}
|
||
```
|
||
|
||
SO 可实现验证逻辑，但缺少配套的 Editor 工具（ValidationRunner），接口孤立存在。
|
||
|
||
#### ✅ CreateAssetMenu 全面覆盖
|
||
|
||
所有 SO 类均标注 `[CreateAssetMenu(menuName = "...")]`，层级清晰，设计师无需记住 SO 路径。
|
||
|
||
---
|
||
|
||
### 5.2 问题
|
||
|
||
#### ⚠️ P1：IValidatable 没有 Runner/Window
|
||
|
||
接口定义完整，但没有对应的 `SOValidationRunner`（Editor 菜单一键验证所有 SO）或 `[MenuItem("Tools/Validate All SOs")]` 实现。目前 `Validate()` 方法从未被调用，等同于死代码。
|
||
|
||
**商业标准：** 《Hades》《Dead Cells》均有数据验证管道，CI 中自动跑，防止设计师填写空/越界数据导致运行时崩溃。
|
||
|
||
#### ⚠️ P1：PlayerController Inspector 字段过多（约 18 个 SerializeField）
|
||
|
||
```csharp
|
||
[Header("核心组件")] // 3个
|
||
[Header("配置")] // 5个
|
||
[Header("战斗")] // 9个
|
||
[Header("事件频道")] // 3个
|
||
```
|
||
|
||
18 个字段全部需要在 Inspector 中手动拖拽赋值，其中 `_movement`、`_stats`、`_animancer`、`_inputBuffer` 均挂在同一 GameObject 上，理应通过 `[RequireComponent]` + `Awake` 自动获取。
|
||
|
||
**建议：** 同节点组件改为 `RequireComponent` + Awake GetComponent；跨节点引用保留 `[SerializeField]`。可将约 18 个字段减至 8–10 个，降低出错概率。
|
||
|
||
#### ⚠️ P2：PostProcessManager 使用 Component 类型代替具体 Volume 类型
|
||
|
||
```csharp
|
||
[SerializeField] private Component _bossArenaVolume; // Assign a Volume component
|
||
```
|
||
|
||
Inspector 中接受任意 `Component`，没有类型约束，设计师可能误拖 `Transform` 或 `Collider`。运行时通过类型转换才能发现错误。
|
||
|
||
**建议：** 改为 `UnityEngine.Rendering.Volume` 或具体后处理 Volume 类型。
|
||
|
||
#### ⚠️ P3：HurtBox Inspector 字段在代码中注入而非序列化
|
||
|
||
```csharp
|
||
// HurtBox.cs
|
||
public void SetShieldable(IShieldable shieldable) => _shieldable = shieldable;
|
||
public void SetParrySystem(ParrySystem ps) => _parrySystem = ps;
|
||
public void SetPoiseSource(IPoiseSource src) => _poiseSource = src;
|
||
```
|
||
|
||
这些依赖在 `PlayerController.Awake()` 中手动注入，Inspector 不可见，也不能提前验证是否遗漏配置。若在另一个 Prefab 上挂 HurtBox 而忘记注入，会静默运行（弹反/霸体失效）。
|
||
|
||
---
|
||
|
||
## 6. 使用便利性（DX）
|
||
|
||
### 6.1 优点
|
||
|
||
#### ✅ 8 步伤害流水线注释完整
|
||
|
||
```csharp
|
||
// 1. 无敌帧检查
|
||
// 2. 弹反检查
|
||
// 3. 霸体检查
|
||
// 4. 护盾层拦截
|
||
// 5. 计算 FinalDamage
|
||
// 6. 调用 _owner.TakeDamage
|
||
// 7. 全局广播
|
||
// 8. 状态效果触发
|
||
```
|
||
|
||
任何开发者接手都能在 10 秒内理解伤害全流程，维护成本低。
|
||
|
||
#### ✅ InputReaderSO 统一输入接口
|
||
|
||
```csharp
|
||
public event Action AttackEvent;
|
||
public event Action DashEvent;
|
||
public event Action ParryEvent;
|
||
public Vector2 MoveInput { get; private set; } // Polling 接口
|
||
```
|
||
|
||
状态类既可订阅事件（`Input.AttackEvent += ...`），也可每帧轮询 `Input.MoveInput`，灵活性高。Gameplay / UI 两套 ActionMap 在 SO 内部切换，调用方无感知。
|
||
|
||
#### ✅ PlayerStateBase 便捷属性减少模板代码
|
||
|
||
```csharp
|
||
protected InputReaderSO Input => _owner.Input;
|
||
protected AnimancerComponent Anim => _owner.Animancer;
|
||
protected PlayerAnimationConfigSO AnimCfg => _owner.AnimConfig;
|
||
```
|
||
|
||
每个状态类中直接写 `Anim.Play(AnimCfg.Dash)` 即可，无需每次手动访问 Owner 链式属性。
|
||
|
||
#### ✅ FormController 形态切换三路广播
|
||
|
||
```csharp
|
||
_onFormChanged?.Raise(index); // SO 事件 → UI/Save
|
||
OnFormChanged?.Invoke(); // C# 事件 → WeaponManager
|
||
_onSkillSetChanged?.Raise(); // SO 事件 → SkillHUD
|
||
```
|
||
|
||
一次切换操作自动通知所有关心形态的子系统，无需调用方手动串联。
|
||
|
||
---
|
||
|
||
### 6.2 问题
|
||
|
||
#### ⚠️ P1：SaveManager 公开 `public SaveData Data` 属性
|
||
|
||
```csharp
|
||
public SaveData Data => _current;
|
||
```
|
||
|
||
外部代码（`ProgressLock`、`HPContainerPickup`）可直接读写 `SaveManager.Instance.Data.XXX`，绕过 SaveManager 的所有访问控制和校验逻辑。`_current` 可能为 null（游戏未读档状态），导致 NullReferenceException。
|
||
|
||
**建议：** 提供具名访问方法（`GetPlayerData()` / `GetWorldData()`），内部做 null guard，移除 `Data` 属性。
|
||
|
||
#### ⚠️ P1：`_dependenciesReady` 标志存在但每帧仍调用检查
|
||
|
||
```csharp
|
||
private void Update()
|
||
{
|
||
if (!HasRequiredStateDependencies()) return; // 每帧调用
|
||
...
|
||
}
|
||
private bool HasRequiredStateDependencies()
|
||
{
|
||
if (_dependenciesReady) return true; // 快速返回
|
||
...
|
||
}
|
||
```
|
||
|
||
`_dependenciesReady` 快速 return 已经优化了大部分开销，但 `Update` 中每帧仍有方法调用栈开销。若依赖项永远在 `Awake` 期间就绪，该方法根本不应在 `Update` 中调用。
|
||
|
||
**建议：** 若 `Awake` 中 `ResolveDependencies()` 后直接断言依赖完整，`Update` 中不再调用检查。仅在开发期 `Awake` 末尾用 `Debug.Assert` 保护。
|
||
|
||
#### ⚠️ P2：QuestManager 的 `OnEnemyDied` 订阅类型不匹配
|
||
|
||
```csharp
|
||
[SerializeField] private TransformEventChannelSO _onEnemyDied; // 携带 Transform
|
||
```
|
||
|
||
敌人死亡事件携带 `Transform`（敌人的 Transform），但任务系统关心的是"哪种敌人死亡"（通常是 EnemyId / EnemyType）。通过 Transform 反向查找 `EnemyBase` 组件获取 ID，多了一层不必要的间接层。
|
||
|
||
**建议：** 死亡事件频道改为携带 `EnemyDeathData { string EnemyId; Transform Source; }`，或改用 `StringEventChannelSO` 直接携带 `EnemyId`。
|
||
|
||
#### ⚠️ P2：SwimState 存在但未在 PlayerController.InitializeStates() 中注册
|
||
|
||
```
|
||
Assets/Scripts/Player/States/SwimState.cs ← 文件存在
|
||
```
|
||
|
||
```csharp
|
||
// PlayerController.InitializeStates()
|
||
_states[typeof(IdleState)] = new IdleState(this);
|
||
// ... 16 个状态，无 SwimState
|
||
```
|
||
|
||
`SwimState` 无法通过 `GetState<SwimState>()` 获取，即使被引用也会返回 null，使游泳状态完全无效。
|
||
|
||
#### ⚠️ P3：PlatformBootstrap `Update()` 每帧调用 `GetOrDefault`
|
||
|
||
```csharp
|
||
private void Update()
|
||
{
|
||
ServiceLocator.GetOrDefault<IPlatformService>()?.RunCallbacks();
|
||
}
|
||
```
|
||
|
||
每帧一次 Dictionary 查找。应在 `Awake` 缓存引用：
|
||
|
||
```csharp
|
||
private IPlatformService _platform;
|
||
private void Awake() { ...; _platform = ServiceLocator.Get<IPlatformService>(); }
|
||
private void Update() => _platform?.RunCallbacks();
|
||
```
|
||
|
||
---
|
||
|
||
## 7. 问题优先级汇总表
|
||
|
||
| # | 优先级 | 模块 | 描述 | 影响 |
|
||
|---|--------|------|------|------|
|
||
| 1 | **P1** | 全局 | 单例 vs ServiceLocator 混用 | 可测试性、生命周期管理 |
|
||
| 2 | **P1** | EnemyBase | ForceState() 空实现，受击无视觉/计时反馈 | 战斗手感 |
|
||
| 3 | **P1** | PlayerMovement | localScale 翻转朝向 | 物理/视觉副作用 |
|
||
| 4 | **P1** | GameServiceRegistrar | 场景加载时 FindObjectsOfType | 加载性能 |
|
||
| 5 | **P1** | Editor | IValidatable 无 Runner | 数据质量保障缺失 |
|
||
| 6 | **P1** | PlayerController | Inspector 18 个 SerializeField | 配置错误概率高 |
|
||
| 7 | **P1** | SaveManager | `Data` 属性绕过访问控制 | 潜在 NullReferenceException |
|
||
| 8 | **P2** | UIManager | 商店 ID 参数被丢弃 | 多商店扩展时需重构 |
|
||
| 9 | **P2** | WorldStateRegistry | 世界状态扩展性差（每类单独字段） | 添加新实体类型成本高 |
|
||
| 10 | **P2** | SkillManager | 固定三技能槽硬编码 | 技能系统扩展瓶颈 |
|
||
| 11 | **P2** | EnemyBase | "Phase" 注释残留，死代码风险 | 维护性 |
|
||
| 12 | **P2** | PlayerController | SwimState 未注册 | 游泳功能完全失效 |
|
||
| 13 | **P2** | QuestManager | EnemyDied 事件携带 Transform 而非 EnemyId | 语义歧义 |
|
||
| 14 | **P2** | GlobalObjectPool | Coroutine + async Task 双版本重复逻辑 | 维护成本翻倍 |
|
||
| 15 | **P3** | PlatformBootstrap | Update 中 ServiceLocator.GetOrDefault 每帧查找 | 微性能 |
|
||
| 16 | **P3** | PostProcessManager | Component 类型引用无约束 | Inspector 配置错误风险 |
|
||
| 17 | **P3** | LocalizationManager | 异常静默吞掉 | 本地化问题难排查 |
|
||
| 18 | **P3** | HurtBox | 依赖注入通过代码而非 Inspector | 编辑器不可见，遗漏静默失效 |
|
||
|
||
---
|
||
|
||
## 8. 优化建议
|
||
|
||
### 8.1 高价值、低成本（1–3天）
|
||
|
||
**① 注册 SwimState**
|
||
|
||
```csharp
|
||
// PlayerController.InitializeStates() 添加一行
|
||
_states[typeof(SwimState)] = new SwimState(this);
|
||
```
|
||
|
||
**② EnemyBase.ForceState() 补充动画与计时**
|
||
|
||
```csharp
|
||
public void ForceState(EnemyStateType newState)
|
||
{
|
||
_currentState = newState;
|
||
switch (newState)
|
||
{
|
||
case EnemyStateType.Hurt:
|
||
if (_animancer != null && _animConfig?.Hurt != null)
|
||
_animancer.Play(_animConfig.Hurt);
|
||
// TODO: 启动硬直计时器
|
||
break;
|
||
case EnemyStateType.Dead:
|
||
Die();
|
||
break;
|
||
}
|
||
}
|
||
```
|
||
|
||
**③ 修复 LocalizationManager 异常日志**
|
||
|
||
```csharp
|
||
catch (Exception e)
|
||
{
|
||
Debug.LogWarning($"[Localization] Key '{entryKey}' in table '{tableName}' failed: {e.Message}");
|
||
return entryKey;
|
||
}
|
||
```
|
||
|
||
**④ PlatformBootstrap 缓存服务引用**
|
||
|
||
```csharp
|
||
private IPlatformService _platform;
|
||
private void Awake() { /* ... init ... */ _platform = ServiceLocator.Get<IPlatformService>(); }
|
||
private void Update() => _platform?.RunCallbacks();
|
||
```
|
||
|
||
**⑤ UIManager 保存并使用商店 ID**
|
||
|
||
```csharp
|
||
private void OpenShop(string shopId)
|
||
{
|
||
// TODO: 根据 shopId 选择对应 ShopController
|
||
OpenPanel(_shopRoot);
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 8.2 中等成本（1–2周）
|
||
|
||
**⑥ PlayerMovement 朝向改为 SpriteRenderer.flipX**
|
||
|
||
```csharp
|
||
// 替换 localScale 翻转
|
||
private SpriteRenderer _sprite;
|
||
private void Awake() => _sprite = GetComponentInChildren<SpriteRenderer>();
|
||
public void UpdateFacing()
|
||
{
|
||
float vx = _rb.velocity.x;
|
||
if (Mathf.Abs(vx) < 0.1f) return;
|
||
int dir = vx > 0f ? 1 : -1;
|
||
if (dir == _facingDirection) return;
|
||
_facingDirection = dir;
|
||
if (_sprite != null) _sprite.flipX = dir < 0;
|
||
}
|
||
```
|
||
|
||
**⑦ PlayerController 减少 Inspector 字段**
|
||
|
||
为同节点组件添加 `[RequireComponent]` 并在 Awake 自动获取：
|
||
```csharp
|
||
[RequireComponent(typeof(PlayerMovement))]
|
||
[RequireComponent(typeof(PlayerStats))]
|
||
[RequireComponent(typeof(AnimancerComponent))]
|
||
[RequireComponent(typeof(InputBuffer))]
|
||
// Awake 中删除手动赋值，直接 GetComponent
|
||
```
|
||
|
||
**⑧ 实现 SOValidationRunner**
|
||
|
||
```csharp
|
||
// Editor/SOValidationRunner.cs
|
||
[MenuItem("Tools/Validate All SOs")]
|
||
public static void ValidateAll()
|
||
{
|
||
var assets = AssetDatabase.FindAssets("t:ScriptableObject");
|
||
foreach (var guid in assets)
|
||
{
|
||
var so = AssetDatabase.LoadAssetAtPath<ScriptableObject>(
|
||
AssetDatabase.GUIDToAssetPath(guid));
|
||
if (so is IValidatable v)
|
||
foreach (var msg in v.Validate())
|
||
Debug.LogWarning($"[Validation] {so.name}: {msg}", so);
|
||
}
|
||
}
|
||
```
|
||
|
||
**⑨ WorldStateRegistry 泛化 API**
|
||
|
||
```csharp
|
||
public enum WorldObjectCategory { Collectible, Door, Destroyed, SavePoint, Flag }
|
||
private readonly Dictionary<WorldObjectCategory, HashSet<string>> _states = new();
|
||
|
||
public bool IsMarked(WorldObjectCategory cat, string id)
|
||
=> _states.TryGetValue(cat, out var set) && set.Contains(id);
|
||
public void Mark(WorldObjectCategory cat, string id)
|
||
{
|
||
if (!_states.ContainsKey(cat)) _states[cat] = new HashSet<string>();
|
||
_states[cat].Add(id);
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 8.3 长期架构改进（迭代计划）
|
||
|
||
**⑩ 统一全局管理器访问模式**
|
||
|
||
选择一种模式，全面推广：
|
||
|
||
- **方案 A（推荐）**：将 `SaveManager`、`QuestManager`、`MapManager` 的 `static Instance` 替换为 `ServiceLocator.Register<ISaveManager>(this)`，所有调用方通过 `ServiceLocator.Get<ISaveManager>()` 访问
|
||
- **方案 B**：保留单例，但所有单例实现共同接口，提供 `ServiceLocator.OverrideForTest<>()` 的测试路径
|
||
|
||
**⑪ 敌人状态机升级**
|
||
|
||
基于现有 Behavior Designer 行为树，完全放弃 `EnemyStateType` 枚举状态机，敌人所有状态逻辑都在 BD Task 中实现。`EnemyBase` 仅提供数据接口（HP、位置、视野），成为纯粹的"数据持有者 + Unity 生命周期桥接器"。
|
||
|
||
**⑫ 单元测试接入**
|
||
|
||
ServiceLocator 已支持 `OverrideForTest` / `Reset`，可以对以下关键系统编写 NUnit 测试：
|
||
- `SaveMigrator.Migrate()` 各版本迁移路径
|
||
- `StatusEffectManager` 堆叠 / 净化逻辑
|
||
- `QuestManager` 目标进度计算
|
||
- `HurtBox.ReceiveDamage()` 8 步流水线各分支
|
||
|
||
---
|
||
|
||
## 附：本次评审同步修复的 Bug
|
||
|
||
> 以下问题在撰写本文档时发现并已直接修复：
|
||
|
||
| 文件 | 问题 | 修复 |
|
||
|------|------|------|
|
||
| `EnemyBase.cs` | `Awake()` 中缺少 `_onPlayerSpawned.OnEventRaised += SetPlayerTransform` 订阅（上轮优化遗漏）；`OnDestroy` 有取消订阅但从未订阅，造成无效的 `-=` 调用 | 已添加订阅，移除 Phase 1 `FindWithTag` 残留代码 |
|
||
|
||
---
|
||
|
||
*文档结束。如需针对任何具体问题深入讨论实施方案，可继续追问。*
|