584 lines
26 KiB
Markdown
584 lines
26 KiB
Markdown
# zeling_v2 高级代码评审报告
|
||
|
||
> **评审标准**：参照成熟商业 2D 动作游戏代码品质（Team Cherry《Hollow Knight》/ Extremely OK Games《Celeste》/ DeNA 商业项目级别）。
|
||
> **版本**：Post-Improvement v1 （第一轮5项改进完成后）
|
||
> **日期**：2025-07
|
||
|
||
---
|
||
|
||
## 执行摘要
|
||
|
||
`zeling_v2` 是一个具备相当架构意识的 Unity 2D 动作游戏项目：程序集分离、ScriptableObject 事件频道、POCO 玩家状态机、接口驱动的战斗管道——这些均超越了大多数同类个人项目。然而，与真正成熟的商业产品相比，在**一致性、性能细节、可测试性、编辑器工具化**等方面仍存在可察觉的差距。
|
||
|
||
| 评审维度 | 评分（/10） | 简评 |
|
||
|---|---|---|
|
||
| 架构设计 | **7.5** | 理念先进，但关键一致性漏洞 |
|
||
| 性能 | **6.5** | 战斗层 GC 友好；全局有5+个性能热点未处理 |
|
||
| 可扩展性 | **7.0** | 主干可扩展；多处硬编码/魔法数字限制扩展 |
|
||
| 编辑器友好性 | **6.0** | 有 EventBusMonitor；缺少 Custom Editor/Gizmo |
|
||
| 使用便利性 | **6.5** | API 设计清晰；错误处理不完善，异步陷阱明显 |
|
||
| **综合** | **6.7** | 远超原型质量，距离商业发行质量尚有差距 |
|
||
|
||
---
|
||
|
||
## 一、架构设计
|
||
|
||
### 1.1 亮点
|
||
|
||
#### ✅ 程序集分离（Assembly Definition）
|
||
25 个 `.asmdef` 文件构成清晰的依赖图（Core ← Combat ← Player ← …），在大型项目中能显著缩短增量编译时间，并强制接口边界。这是正确的工程实践。
|
||
|
||
#### ✅ ScriptableObject 事件频道 + CompositeDisposable
|
||
`BaseEventChannelSO<T>` + `EventSubscription` + `CompositeDisposable` 提供了可 Inspector 连线、零运行时依赖的事件总线，并通过 `AddTo()` 扩展方法实现生命周期安全的订阅。设计参考了 ReactiveX 风格，是目前团队规模下的优良选择。
|
||
|
||
#### ✅ POCO 玩家状态机
|
||
`PlayerStateBase` 继承链 + `PlayerController` 的 `Dictionary<Type, PlayerStateBase>` 注册表实现了真正的无 MonoBehaviour 状态机，可以充分利用 C# 类型系统，比 Animator State Machine 更可测试。
|
||
|
||
#### ✅ 战斗管道接口隔离
|
||
`HurtBox` 通过 `SetShieldable() / SetParrySystem() / SetPoiseSource()` 注入接口，而非直接依赖具体类；`HitBox` 通过 `IBreakable` 接口处理可破坏物体——这消除了跨程序集硬耦合。
|
||
|
||
#### ✅ ServiceLocator + 注册器分离
|
||
`GameServiceRegistrar`（`DefaultExecutionOrder(-2000)`）统一在最早阶段注册服务，`ServiceLocator.RegisterIfAbsent<IAudioService>(new NullAudioService())` 的空对象模式（Null Object Pattern）值得称赞。
|
||
|
||
---
|
||
|
||
### 1.2 关键问题
|
||
|
||
#### ❌【严重】ServiceLocator 一致性漏洞
|
||
代码库中同时存在两套服务定位机制：
|
||
|
||
| 组件 | 访问方式 | 问题 |
|
||
|---|---|---|
|
||
| `ClashResolver` | `ServiceLocator.Register<ClashResolver>` | ✅ 正确 |
|
||
| `DifficultyManager` | `ServiceLocator.Register<DifficultyManager>` | ✅ 正确 |
|
||
| `GameManager` | `public static Instance` | ❌ 与理念相悖 |
|
||
| `SaveManager` | `public static Instance` | ❌ 与理念相悖 |
|
||
| `GlobalObjectPool` | `public static Instance` | ❌ 与理念相悖 |
|
||
|
||
三个核心 Manager 保留了传统 Singleton，导致测试时无法注入 Mock 实现，也使 `ServiceLocator` 的价值大打折扣。商业项目通常要求风格统一，否则维护者永远不确定"该用哪种方式查找服务"。
|
||
|
||
**建议**：将三者改为通过 `ServiceLocator.Register<ISaveManager>` / `IGameManager` / `IObjectPool` 注册，并在 `GameServiceRegistrar.Awake()` 统一入口注册。
|
||
|
||
---
|
||
|
||
#### ❌【严重】`EnemyBase.Awake()` 使用 `FindWithTag("Player")`
|
||
```csharp
|
||
// EnemyBase.cs:162
|
||
var playerGO = GameObject.FindWithTag("Player");
|
||
if (playerGO != null) _playerTransform = playerGO.transform;
|
||
```
|
||
场景中有 N 个敌人时，`Awake()` 被调用 N 次，每次执行全场景标签扫描（O(n) 场景对象遍历）。在 Hollow Knight / Celeste 这类商业项目中，此类查找统一通过事件频道推送或注册表注入。
|
||
|
||
**建议**：在 `GameServiceRegistrar` 或 `PlayerController.Awake()` 完成后，通过 `TransformEventChannelSO` 广播玩家 Transform；所有敌人订阅该事件接收引用。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`GameStateMachine` 缺乏守卫条件（Guard Condition）
|
||
```csharp
|
||
// GameStateMachine.cs:29
|
||
if (_current != null && !_current.ValidNextStates.Contains(nextId))
|
||
```
|
||
`ValidNextStates` 是运行时列表检查，属于防御式断言而非真正的状态转换守卫。商业质量的状态机通常为每个转换弧定义独立的 `CanTransitionTo(context)` 谓词，以便同一目标状态在不同条件下可以允许或拒绝转换，逻辑集中在转换定义上，而非分散在调用方。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`EquipmentManager` 直接使用 `EventChannelRegistry.Instance`
|
||
```csharp
|
||
// EquipmentManager.cs:39
|
||
Events = EventChannelRegistry.Instance,
|
||
```
|
||
这是另一个静态 Singleton 访问点，绕过了 `ServiceLocator`，且破坏了 `EquipmentManager` 的可测试性。
|
||
|
||
---
|
||
|
||
### 1.3 依赖方向图（实际 vs 理想）
|
||
|
||
```
|
||
理想:
|
||
GameServiceRegistrar → ServiceLocator ← [所有使用方]
|
||
|
||
实际:
|
||
GameServiceRegistrar → ServiceLocator ← [部分使用方]
|
||
↑
|
||
GameManager.Instance ←─────────┘ （绕过）
|
||
SaveManager.Instance ←─────────┘ （绕过）
|
||
EventChannelRegistry.Instance ←┘ （绕过）
|
||
```
|
||
|
||
---
|
||
|
||
## 二、性能
|
||
|
||
### 2.1 亮点
|
||
|
||
#### ✅ `DamageInfo` 零 GC 设计
|
||
`DamageInfo` 为 `struct` + `DamageInfo.From()` 工厂方法，在战斗最热路径上避免堆分配。在帧率敏感的动作游戏中，这是正确的优先选择。
|
||
|
||
#### ✅ `StatusEffectManager` 双结构（List + Dictionary）
|
||
`_activeList`（O(1) Update 遍历）+ `_activeIndex`（O(1) 类型查找）是标准的游戏引擎 ECS 风格优化，避免了在 Update 中进行字典遍历。
|
||
|
||
#### ✅ `GlobalObjectPool` LRU 回收 + Addressables 预热
|
||
池空时的 Background Refill Coroutine + Addressables 异步加载是成熟的生产级对象池模式。
|
||
|
||
#### ✅ Animancer 帧事件驱动 HitBox
|
||
帧事件（`events.Add(enterTime, ...)` / `events.Add(exitTime, ...)`）比 `Physics.OverlapCircle` 轮询更精确，也避免了每帧碰撞查询。
|
||
|
||
---
|
||
|
||
### 2.2 性能问题
|
||
|
||
#### ❌【高危】`PlayerController.FindDefaultInputReader()` 全资产扫描
|
||
```csharp
|
||
// PlayerController.cs（推测）
|
||
Resources.FindObjectsOfTypeAll<InputReaderSO>()
|
||
```
|
||
`Resources.FindObjectsOfTypeAll<T>()` 扫描内存中**所有已加载对象**（包括所有 Asset），在大型项目的 `Start()` 期间可造成 10~50ms 卡顿（取决于资产数量）。
|
||
|
||
**建议**：通过 `[SerializeField]` 在 Inspector 直接赋值；若需运行时发现，改用 `ServiceLocator.Get<InputReaderSO>()` 或在 `GameServiceRegistrar` 中注册。
|
||
|
||
---
|
||
|
||
#### ❌【高危】`PlayerController` 每帧调用 `ResolveDependencies()`
|
||
```csharp
|
||
// 推测：Update() 和 Start() 均调用 HasRequiredStateDependencies()
|
||
// HasRequiredStateDependencies() 内部调用 ResolveDependencies()
|
||
```
|
||
依赖解析在 `Start()` 完成后结果不会改变，但代码每帧 `Update()` 中重复执行。在玩家控制器（每帧必定执行）中，这是纯粹的冗余 CPU 周期。
|
||
|
||
**建议**：`_dependenciesResolved` 布尔标志，仅在首次成功后设 `true`，后续 `Update()` 跳过检查。
|
||
|
||
---
|
||
|
||
#### ❌【高危】`HurtBox.ReceiveDamage()` 每次受击调用 `GetComponent<>()`
|
||
```csharp
|
||
// HurtBox.cs:105
|
||
if (_owner is MonoBehaviour mb)
|
||
mb.GetComponent<IStatusEffectable>()?.ApplyStatusEffect(info.Type);
|
||
```
|
||
`GetComponent<>()` 代价比属性访问高约 10-50 倍（内部有 NativeArray 遍历）。在快节奏战斗中玩家/敌人每秒可能受击 5~10 次。
|
||
|
||
**建议**：在 `Awake()` 缓存 `_statusEffectable = GetComponentInParent<IStatusEffectable>()`。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`EnemyBase.Update()` 每帧计算所有敌人到玩家距离
|
||
```csharp
|
||
// EnemyBase.cs:180
|
||
_stats.DistanceToPlayer = Vector2.Distance(transform.position, _playerTransform.position);
|
||
```
|
||
场景中 50 个敌人 = 每帧 50 次 `Vector2.Distance`（含 `sqrt`）。可优化为 `SqrMagnitude` 比较（避免 sqrt），或通过 `BatchLOSSystem` 分帧分批处理。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`SaveManager` 双重序列化 + HMAC 计算
|
||
```csharp
|
||
// SaveManager.cs:62-67
|
||
string jsonForChecksum = JsonConvert.SerializeObject(_current, Formatting.Indented);
|
||
_current.Meta.Checksum = ComputeChecksum(jsonForChecksum);
|
||
string finalJson = JsonConvert.SerializeObject(_current, Formatting.Indented);
|
||
```
|
||
每次存档执行两次完整 JSON 序列化（第二次含 Checksum 字段）。`Formatting.Indented` 会产生更大的字符串对象（大量 GC）。存档是低频操作，主要问题是 `Formatting.Indented` 的 GC 压力。
|
||
|
||
**建议**：生产构建用 `Formatting.None`；Checksum 可改为只计算关键字段而非全量 JSON。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`GlobalObjectPool._alive` 列表头删除 O(n)
|
||
```csharp
|
||
// GlobalObjectPool.cs:120
|
||
po = aliveList[0];
|
||
aliveList.RemoveAt(0); // O(n) — 移动后续所有元素
|
||
```
|
||
LRU 回收时 `RemoveAt(0)` 需要移动列表中所有后续元素。若活跃对象较多（>20），累积开销明显。
|
||
|
||
**建议**：改用 `LinkedList<PooledObject>` 或循环队列（`Queue<PooledObject>`）实现 O(1) LRU。
|
||
|
||
---
|
||
|
||
#### ⚠️【低危】`StatusEffectManager.Update()` `RemoveAt` vs `_activeList.Remove`
|
||
代码注释说"逆序遍历避免索引错位"，但若 `RemoveAt(i)` 调用的是索引删除（而非 `_activeList.Remove(effect)`），则性能更优，逻辑也更清晰。需确认实际调用的是 `_activeList.RemoveAt(i)` 而非线性搜索的 `_activeList.Remove(effect)`。
|
||
|
||
---
|
||
|
||
## 三、可扩展性
|
||
|
||
### 3.1 亮点
|
||
|
||
#### ✅ `CharmSO` + Effect 组合模式
|
||
护符效果通过 `CharmEffectSO` 抽象 + `effects[]` 数组实现组合（Composite Pattern），新增护符效果只需新建 ScriptableObject 子类，无需修改 `EquipmentManager`。这是 Hollow Knight 类游戏护符系统的正确实现方式。
|
||
|
||
#### ✅ `IGameState` + `GameStateMachine` 状态注册表
|
||
游戏状态通过接口注册，新增游戏状态不需要修改状态机本身（Open/Closed 原则）。
|
||
|
||
#### ✅ `ISaveable` + `SaveManager.Register/Unregister`
|
||
存档系统通过接口订阅而非硬编码字段，新增可存档组件只需实现 `ISaveable`，SaveManager 不感知具体类型。
|
||
|
||
---
|
||
|
||
### 3.2 可扩展性问题
|
||
|
||
#### ❌【高危】`AttackState` 连击数硬编码
|
||
```csharp
|
||
// AttackState.cs:61
|
||
if (_comboIndex < 2) // 魔法数字：限制为 3 段连击
|
||
```
|
||
若需要不同武器/形态有不同连击数，必须修改 `AttackState` 源码。商业质量的连击系统通常把上限配置在 `PlayerAnimationConfigSO.GroundAttacks.Length` 中，动态读取。
|
||
|
||
**建议**：
|
||
```csharp
|
||
if (_comboIndex < AnimCfg.GroundAttacks.Length - 1)
|
||
```
|
||
|
||
---
|
||
|
||
#### ❌【高危】`HurtState` 硬编码持续时间
|
||
```csharp
|
||
// HurtState.cs:15
|
||
private const float HurtDuration = 0.4f;
|
||
```
|
||
不同敌人攻击的硬直时长应可配置，但 `HurtState` 不接受外部参数。若特定攻击需要更长/更短的硬直，当前架构无法支持，只能修改常量或添加新状态。
|
||
|
||
**建议**：`Initialize(DamageInfo info)` 已存在，将 `HurtDuration` 改为从 `info` 或 `PlayerAnimationConfigSO` 读取。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`WorldStateRegistry` 多独立 HashSet 硬编码
|
||
```csharp
|
||
// WorldStateRegistry.cs
|
||
private HashSet<string> _collectedIds = new();
|
||
private HashSet<string> _activatedSavePoints = new();
|
||
private HashSet<string> _openedDoors = new();
|
||
private HashSet<string> _destroyedObjects = new();
|
||
private HashSet<string> _flags = new();
|
||
```
|
||
每种世界状态类型都需要修改 `WorldStateRegistry` 类本身。若新增"已解锁秘密房间"类别，需添加新字段、新方法、新序列化逻辑。
|
||
|
||
**建议**：统一用 `Dictionary<WorldStateType, HashSet<string>>` 或单一 `_flags` 加命名前缀约定（`"door:xxx"` / `"collect:xxx"`）。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`InputReaderSO` 无输入优先级/拦截机制
|
||
`InputReaderSO` 直接暴露 C# `Action` 事件，没有优先级层、拦截点或输入消费标记。当对话/UI 弹出时，需要手动调用 `EnableUIInput()` / `DisableGameplayInput()`，且整个 Gameplay map 被禁用。这在有多个输入消费方（技能、背包、对话）的游戏中容易产生竞争。
|
||
|
||
**参考**：Celeste 使用 InputManager 层级 + 输入消费标记（consumed flag）防止事件穿透。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`EnemyBase.ForceState()` 无完整状态回调
|
||
```csharp
|
||
// EnemyBase.cs:153
|
||
public void ForceState(EnemyStateType newState)
|
||
{
|
||
_currentState = newState;
|
||
// Phase 2：根据状态播放对应动画 / 触发硬直计时
|
||
}
|
||
```
|
||
`ForceState` 没有 `OnExit` / `OnEnter` 回调。当前用 `// Phase 2` 注释占位，但这意味着：敌人的任何行为改变（动画切换、计时器重置、Navigation 停止）都需要调用方自己处理，极易出现漏调。
|
||
|
||
**建议**：即使 Phase 1 简化实现，也应在 `ForceState` 内置虚方法调用框架：
|
||
```csharp
|
||
_current?.OnExit();
|
||
_currentState = newState;
|
||
OnEnemyStateEnter(newState);
|
||
```
|
||
|
||
---
|
||
|
||
## 四、编辑器友好性
|
||
|
||
### 4.1 亮点
|
||
|
||
#### ✅ `EventBusMonitor`（`#if UNITY_EDITOR`）
|
||
在 `BaseEventChannelSO.Raise()` 中集成编辑器监控，可以追踪每个事件频道的触发历史和订阅者数量，是非常实用的调试工具。
|
||
|
||
#### ✅ `[DefaultExecutionOrder]` 明确声明
|
||
各组件明确声明执行顺序（-2000 / -1000 / -900 / -800 / -100），避免了 Unity 脚本执行顺序的隐式依赖，易于维护。
|
||
|
||
#### ✅ `[Header]` / `[SerializeField]` 组织清晰
|
||
各 MonoBehaviour 使用 `[Header]` 分组序列化字段，Inspector 布局清晰。
|
||
|
||
#### ✅ `GlobalObjectPool.PoolConfig` 可序列化结构
|
||
`[Serializable] PoolConfig` 使对象池配置可在 Inspector 中编辑，比硬编码 string 数组更安全。
|
||
|
||
---
|
||
|
||
### 4.2 编辑器友好性问题
|
||
|
||
#### ❌【高危】`PlayerAnimationConfigSO` 的 `float[]` 帧事件时间
|
||
```csharp
|
||
// AttackState.cs:42-46
|
||
float enterTime = AnimCfg?.GroundAttackHitBoxEnterTimes?[_comboIndex] ?? 0.3f;
|
||
float exitTime = AnimCfg?.GroundAttackHitBoxExitTimes?[_comboIndex] ?? 0.6f;
|
||
```
|
||
`GroundAttackHitBoxEnterTimes` 和 `GroundAttackHitBoxExitTimes` 是 `float[]`，在 Inspector 中显示为纯数字数组，与具体动画帧没有视觉对应关系。美术和设计师在调整 HitBox 时机时必须手动换算归一化时间（0.0~1.0）。
|
||
|
||
这与 Hollow Knight / Celeste 的帧事件工作流有明显差距——通常这类数据通过专门的 Animancer 帧事件 Inspector 工具（Timeline 预览 + Gizmo）来可视化编辑。
|
||
|
||
**建议**：为 `PlayerAnimationConfigSO` 编写 `CustomPropertyDrawer`，在 Inspector 中显示当前 AnimationClip 帧对应的归一化时间；或集成 Animancer 的 `AnimancerEvent.Sequence` 并在 Inspector 中预览。
|
||
|
||
---
|
||
|
||
#### ❌【中等】缺少关键 Gizmo
|
||
以下关键运行时数据在 Scene 视图中不可见：
|
||
- `HitBox` 的实际碰撞范围（只有 Collider Gizmo，没有攻击伤害信息提示）
|
||
- `EnemyStatsSO.DetectionRange` / `AttackRange` 的可视化圆
|
||
- `BatchLOSSystem`（若存在）的视线射线可视化
|
||
- `GlobalObjectPool` 的活跃/待机对象数量 HUD
|
||
|
||
商业项目（尤其是动作游戏）通常有丰富的 `OnDrawGizmosSelected()` 实现，辅助设计师在不运行游戏的情况下进行关卡设计。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`SaveManager.cs` 缺少存档调试工具
|
||
`SaveManager` 没有编辑器菜单 / Inspector 按钮用于：
|
||
- 在 Inspector 中查看当前 `_current` SaveData 内容
|
||
- 一键清除所有存档槽
|
||
- 手动触发 SaveAsync / LoadAsync
|
||
|
||
对于快速迭代来说，这些工具往往节省大量时间。Hollow Knight 据报道有完整的内部调试存档面板。
|
||
|
||
---
|
||
|
||
#### ⚠️【低危】`EnemyBase` 缺少运行时状态可视化
|
||
`EnemyBase.CurrentState`（`EnemyStateType`）在运行时无法在 Inspector 中直接看到，只能靠断点或 Debug.Log。将 `[field: SerializeField, ReadOnly]` 或通过 Custom Inspector 暴露运行时状态，可大幅提升调试效率。
|
||
|
||
---
|
||
|
||
## 五、使用便利性（开发者体验）
|
||
|
||
### 5.1 亮点
|
||
|
||
#### ✅ `PlayerController.GetState<T>()` 泛型访问器
|
||
```csharp
|
||
Owner.TryTransitionState(Owner.GetState<IdleState>());
|
||
```
|
||
泛型状态访问器比枚举索引更安全（编译时类型检查），比 `as` 转型更简洁。
|
||
|
||
#### ✅ `ISaveable` + `ISaveStorage` 接口分离
|
||
`SaveManager` 不直接依赖文件系统，`ISaveStorage` 可注入 `InMemoryStorage`（测试用）/ `CloudStorage`（扩展用）。良好的 DI 设计。
|
||
|
||
#### ✅ `EquipmentManager.TryEquipCharm()` 返回错误字符串
|
||
```csharp
|
||
public string TryEquipCharm(CharmSO charm) // null=成功，string=失败原因
|
||
```
|
||
返回错误消息而非 bool，调用方（UI）可以直接将错误显示给玩家，无需额外查询失败原因。
|
||
|
||
#### ✅ `CompositeDisposable` + `AddTo()` 链式订阅
|
||
```csharp
|
||
channel.Subscribe(Handler).AddTo(_subscriptions);
|
||
```
|
||
Rx 风格的订阅生命周期管理，在 `OnDisable` 一行 `Clear()` 即可安全取消所有订阅，不会出现常见的订阅泄漏。
|
||
|
||
---
|
||
|
||
### 5.2 使用便利性问题
|
||
|
||
#### ❌【高危】`SaveManager.QuickSave()` Fire-and-Forget 异常吞噬
|
||
```csharp
|
||
// SaveManager.cs:118
|
||
public void QuickSave() => _ = SaveAsync(QuickSaveSlot);
|
||
public void QuickLoad() => _ = LoadAsync(QuickSaveSlot);
|
||
```
|
||
`_ = SaveAsync(...)` 丢弃了 `Task` 对象，任何 `SaveAsync` 内部的异常（包括文件写入失败、序列化异常）都会被**静默吞掉**，不会触发任何错误提示，也不会调用 `Debug.LogError`。
|
||
|
||
在生产环境中这意味着：玩家可能以为存档成功，实际上存档悄悄失败了。
|
||
|
||
**建议**：
|
||
```csharp
|
||
public void QuickSave() => SaveAsync(QuickSaveSlot).LogErrors("[SaveManager] QuickSave failed");
|
||
// 实现扩展方法：
|
||
public static async void LogErrors(this Task task, string msg)
|
||
{
|
||
try { await task; }
|
||
catch (Exception e) { Debug.LogError($"{msg}: {e}"); }
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
#### ❌【高危】`ServiceLocator.Get<T>()` 失败无异常栈
|
||
```csharp
|
||
// ServiceLocator.cs（推测）
|
||
Debug.LogError($"[ServiceLocator] 未找到服务 {typeof(T).Name}");
|
||
return default;
|
||
```
|
||
返回 `default`（null）而非抛出异常，调用方通常不会检查返回值，导致 `NullReferenceException` 在**完全不相关的位置**崩溃，调试时难以定位根本原因。
|
||
|
||
**建议**：提供两种方法：
|
||
```csharp
|
||
public static T Get<T>() // 未找到时抛 ServiceNotRegisteredException
|
||
public static T GetOrDefault<T>() // 未找到时返回 null（已存在）
|
||
```
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`InputReaderSO` 缺少统一"是否按下"轮询 API
|
||
`InputReaderSO` 目前只有 `MoveInput`（`Vector2`）可轮询，其他输入（Jump、Attack、Dash 等）只能通过事件订阅。当状态机需要在 `OnStateUpdate()` 中检查"当前帧 Attack 是否被按住"时，无法通过 SO 轮询，只能通过外部缓冲标志（`InputBuffer`）。
|
||
|
||
设计上并无大错，但不一致性（部分支持轮询，部分不支持）会导致混淆。
|
||
|
||
---
|
||
|
||
#### ⚠️【中等】`PlayerController` 外部状态暴露粒度粗
|
||
`PlayerController` 通过 `GetState<T>()` 暴露所有状态对象，但外部代码（如 UI、Enemy AI）通常只需要询问"玩家是否在攻击中"，而不需要直接访问 `AttackState` 实例。这会导致外部代码对状态对象的直接依赖，破坏封装。
|
||
|
||
**建议**：在 `PlayerController` 上增加语义化查询属性：
|
||
```csharp
|
||
public bool IsAttacking => _currentState is AttackState;
|
||
public bool IsInvincible => Stats.IsInvincible;
|
||
public bool IsGrounded => Move.IsGrounded;
|
||
```
|
||
|
||
---
|
||
|
||
#### ⚠️【低危】`EnemyBase.Die()` 方法缺失（可见调用但未定义）
|
||
在读取的 `EnemyBase.cs` 中，`TakeDamage()` 调用了 `Die()`，但 `Die()` 方法体在阅读的代码段中未找到完整实现（可能在文件末尾）。若 `Die()` 为虚方法但基类无默认实现（只是抛异常或空实现），子类必须知道重写它，否则敌人死亡逻辑静默失败。
|
||
|
||
---
|
||
|
||
## 六、模块级评估
|
||
|
||
### 6.1 战斗系统（HitBox / HurtBox）—— ⭐⭐⭐⭐
|
||
|
||
**优点**：架构上已达商业级水准。
|
||
- DamageInfo struct 零 GC
|
||
- 8步伤害管道完整定义
|
||
- Per-target 冷却（`Dictionary<Collider2D, float>`）避免同帧多次命中
|
||
- `_rivalHitBoxMask` LayerMask 可配置
|
||
|
||
**主要问题**：
|
||
- `HurtBox` 第8步 `GetComponent<IStatusEffectable>()` 每次受击都调用（应缓存）
|
||
- `DamageFlags` 的 `IgnoreIFrame` 设计良好，但缺少文档说明哪些攻击应设置此标志
|
||
|
||
---
|
||
|
||
### 6.2 存档系统（SaveManager）—— ⭐⭐⭐½
|
||
|
||
**优点**：
|
||
- ISaveable 订阅模式（Register/Unregister）
|
||
- HMACSHA256 Checksum（防篡改）
|
||
- SaveMigrator（版本迁移）设计前瞻
|
||
- async Task 全异步 IO
|
||
|
||
**主要问题**：
|
||
- 仍使用 `public static Instance`（Singleton 不一致）
|
||
- `QuickSave()` fire-and-forget 异常吞噬
|
||
- 双重 JSON 序列化（性能浪费）
|
||
- Checksum 密钥用 `SystemInfo.deviceUniqueIdentifier`（设备唯一标识），玩家更换设备时存档将永远校验失败——这是**逻辑 Bug**
|
||
|
||
---
|
||
|
||
### 6.3 对象池（GlobalObjectPool）—— ⭐⭐⭐½
|
||
|
||
**优点**：
|
||
- Addressables 异步预热
|
||
- LRU 回收机制（有上限控制）
|
||
- Background Refill Coroutine
|
||
|
||
**主要问题**：
|
||
- 仍使用 `public static Instance`
|
||
- LRU `RemoveAt(0)` 是 O(n)
|
||
- `GetComponentCached<T>()` 方法未在阅读代码中找到实现（若不存在则是命名不一致）
|
||
|
||
---
|
||
|
||
### 6.4 事件系统（EventChannel）—— ⭐⭐⭐⭐½
|
||
|
||
**优点**：
|
||
- 泛型基类 + 派生具体类型
|
||
- `EventSubscription` + `CompositeDisposable`
|
||
- `EventBusMonitor` 编辑器监控
|
||
|
||
**主要问题**：
|
||
- `OnEventRaised?.GetInvocationList().Length` 在 Editor 监控中：每次 Raise 都分配 `GetInvocationList()` 返回的 `Delegate[]` 数组（即使在 Editor 中）。可以改用 `_listenerCount` 计数器字段避免分配。
|
||
|
||
---
|
||
|
||
### 6.5 敌人系统（EnemyBase）—— ⭐⭐⭐
|
||
|
||
**优点**：
|
||
- IPathAgent 接口隔离（PathBerserker2d 依赖被包装）
|
||
- ILOSRequester 接口支持批量 LOS 计算
|
||
- BD Task 通过虚方法接口访问，避免直接依赖 BehaviorTree API
|
||
|
||
**主要问题**：
|
||
- `FindWithTag("Player")` 全场景扫描（N个敌人 × 场景加载时）
|
||
- `ForceState()` 无状态进入/退出回调
|
||
- `Update()` 每帧 `Vector2.Distance()`
|
||
|
||
---
|
||
|
||
### 6.6 玩家状态机（PlayerController + States）—— ⭐⭐⭐⭐
|
||
|
||
**优点**：
|
||
- POCO 状态机（无 MonoBehaviour 开销）
|
||
- `Dictionary<Type, PlayerStateBase>` 注册 + `GetState<T>()` 泛型访问
|
||
- 构造函数注入（PlayerController → PlayerStateBase）
|
||
|
||
**主要问题**：
|
||
- `IPoiseSource.GetCurrentPoiseLevel()` 硬编码返回 `PoiseLevel.None`
|
||
- `AttackState` 连击数硬编码 `< 2`
|
||
- `HurtState.HurtDuration` 硬编码 `0.4f`
|
||
- `FindDefaultInputReader()` 全资产扫描
|
||
- `DropThroughPlatform()` 空实现（stub）未标注 `[System.Obsolete]` 或 `// TODO` 跟踪
|
||
|
||
---
|
||
|
||
## 七、关键问题清单（优先级排序）
|
||
|
||
| 优先级 | 问题 | 影响范围 | 建议行动 |
|
||
|---|---|---|---|
|
||
| P0 | SaveManager Checksum 用设备ID——换设备必崩 | 存档完整性 | 改为游戏固定密钥或用户密码哈希 |
|
||
| P0 | `QuickSave()` fire-and-forget 异常吞噬 | 存档可靠性 | 添加 `.LogErrors()` 扩展 |
|
||
| P1 | `EnemyBase` `FindWithTag("Player")` N次全扫描 | 场景加载性能 | 事件频道推送 or ServiceLocator 注入 |
|
||
| P1 | `HurtBox` 每次受击 `GetComponent<IStatusEffectable>()` | 战斗帧率 | Awake 缓存 |
|
||
| P1 | `PlayerController` `FindDefaultInputReader()` 全资产扫描 | 游戏启动性能 | SerializeField 直接赋值 |
|
||
| P1 | ServiceLocator / static Instance 不一致（3处） | 架构一致性 | 统一注册入口 |
|
||
| P2 | `AttackState` 连击数硬编码 `< 2` | 扩展性 | 从配置 SO 读取 |
|
||
| P2 | `HurtState.HurtDuration` 硬编码 | 扩展性 | 从 DamageInfo 或 SO 读取 |
|
||
| P2 | `EnemyBase.ForceState()` 无状态回调 | 敌人行为正确性 | 添加 OnEnter/OnExit 框架 |
|
||
| P2 | `GlobalObjectPool` LRU `RemoveAt(0)` O(n) | 运行时性能 | 改用 LinkedList 或循环队列 |
|
||
| P3 | `PlayerAnimationConfigSO` float[] 帧事件无可视化 | 编辑器效率 | 自定义 PropertyDrawer |
|
||
| P3 | 缺少关键 Gizmo（攻击范围、检测范围） | 关卡设计效率 | `OnDrawGizmosSelected()` |
|
||
| P3 | `ServiceLocator.Get<T>()` 失败无异常栈 | 调试效率 | 提供抛异常版本 |
|
||
| P3 | `IPoiseSource.GetCurrentPoiseLevel()` 硬编码返回 None | 功能完整性 | 实现真实的霸体等级查询 |
|
||
| P3 | `WorldStateRegistry` 多独立 HashSet 扩展性差 | 可维护性 | 统一 `Dictionary<WorldStateType, HashSet<string>>` |
|
||
|
||
---
|
||
|
||
## 八、与商业标准差距总结
|
||
|
||
### 已达商业水准 ✅
|
||
- 程序集隔离 + 接口驱动的跨模块通信
|
||
- ScriptableObject 事件频道 + 生命周期安全订阅
|
||
- DamageInfo struct 零 GC 战斗管道
|
||
- 8步伤害流水线（弹反/霸体/护盾/减免）
|
||
- ISaveable + async IO + SaveMigrator + Checksum
|
||
- CharmSO Effect 组合模式
|
||
- Animancer 帧事件驱动 HitBox 时机
|
||
|
||
### 尚未达商业水准 ❌
|
||
- **一致性**：3处核心 Manager 绕过 ServiceLocator
|
||
- **性能细节**：多个热路径 `GetComponent<>()` 未缓存
|
||
- **调试工具**：缺少 Custom Inspector / Gizmo / Runtime Debug HUD
|
||
- **配置数据验证**：float[] 帧事件时间无范围检查（可写入 NaN/负数）
|
||
- **异步安全**：fire-and-forget 导致存档失败静默
|
||
- **平台 Bug**：Checksum 设备ID 跨设备失效
|
||
- **状态机完整性**：Enemy ForceState 无回调、玩家 DropThroughPlatform 空 stub
|
||
- **输入拦截**：无优先级/消费标记机制
|
||
|
||
### 工作量估算（仅关键 P0~P1 修复）
|
||
| 任务 | 估算工时 |
|
||
|---|---|
|
||
| Checksum 密钥修复 | 0.5h |
|
||
| QuickSave fire-and-forget 修复 | 1h |
|
||
| HurtBox GetComponent 缓存 | 0.5h |
|
||
| EnemyBase FindWithTag 替换（含事件频道设计） | 3h |
|
||
| PlayerController InputReader SerializeField | 0.5h |
|
||
| 3处 Instance 迁移至 ServiceLocator | 4h |
|
||
| **合计** | **~9.5h** |
|
||
|
||
---
|
||
|
||
*本报告基于对 `Assets/Scripts/` 下约 20 个关键源文件的直接阅读，覆盖 Core、Combat、Player、Enemies、Save、Equipment、World、Events、Pool 模块。未直接覆盖的模块（Audio、UI、Quest、Cutscene、Localization）需另行评审。*
|