Files
zeling_v2/Docs/Review/CodeQualityReview.md
2026-05-12 15:34:08 +08:00

471 lines
19 KiB
Markdown
Raw Blame History

This file contains invisible Unicode characters
This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 代码质量全面评估报告
> **评估日期**2026-05
> **评估范围**`Assets/Scripts/`25 个程序集,覆盖全部 24 个功能模块)
> **评估标准**:以商业独立/AA 级 2D 动作游戏(如 Hollow Knight、Ori 系列、Dead Cells 技术架构)为基准
> **评估维度**:架构设计、性能、可扩展性、编辑器友好性、使用便利性
---
## 综合评分
| 评估维度 | 评分5 分制) | 简评 |
|--------------|:-----------:|------|
| 架构设计 | ⭐⭐⭐⭐½ | 模块隔离清晰,模式选型专业,少量结构性问题 |
| 性能 | ⭐⭐⭐⭐ | 热路径零 GC、批量 LOS、对象池均到位有几处隐患 |
| 可扩展性 | ⭐⭐⭐⭐ | 接口驱动+SO数据驱动新增功能低耦合主要短板在玩家状态机 |
| 编辑器友好性 | ⭐⭐⭐⭐½ | EventBusMonitor、SO 数据资产、Header 属性完善 |
| 使用便利性 | ⭐⭐⭐⭐ | 依赖注入清晰,部分 API 命名/一致性可改进 |
| **综合** | **⭐⭐⭐⭐** | **接近商业品质,核心问题为单例泛滥和局部硬编码** |
---
## 一、架构设计
### 1.1 程序集划分Assembly Definition✅ 优秀
项目将 25+ 个功能域拆分为独立 `.asmdef` 程序集:
```
BaseGames.Core → BaseGames.Core.Events → BaseGames.Core.Save
BaseGames.Combat → BaseGames.Combat.StatusEffects
BaseGames.Player → BaseGames.Player.States
BaseGames.Enemies → BaseGames.Enemies.AI → BaseGames.Enemies.Navigation
...
```
**优势**
- 增量编译:修改 `BaseGames.Player.States` 不触发 `BaseGames.Core` 重新编译,加快迭代速度
- 强制接口边界:`EnemyBase` 不可直接访问 `PlayerController` 内部,只能通过 `IDamageable`
- 符合《Hollow Knight》及同类商业项目的程序集组织标准
**不足**
- `BaseGames.Core.Events` 程序集中混入了 `DamageInfo``HitInfo` 等战斗领域对象(见 `Core/Events/DamageInfo.cs`),违反了"Core.Events 只存事件通道基础设施"的原则,应移至 `BaseGames.Combat`
---
### 1.2 ServiceLocator 模式 ✅ 专业
```csharp
// 接口类型注册,天然支持依赖倒置
ServiceLocator.Register<IAudioService>(realAudioManager);
ServiceLocator.RegisterIfAbsent<IAudioService>(new NullAudioService()); // Null Object 兜底
```
**优势**
- `RegisterIfAbsent` 防止多场景重复注册,符合 Persistent 场景架构
- `NullAudioService` 是教科书级 Null Object 模式,避免 null 检查污染业务代码
- Editor 下提供 `OverrideForTest` / `Reset` 方法,支持单元测试
- 轻量静态字典,无 DI 框架依赖,适合游戏运行时
**不足**
- 静态类在场景重载时不会自动清空,若忘记在 `GameServiceRegistrar.OnDestroy` 注销,可能持有失效引用(已通过 `DontDestroyOnLoad` 缓解但未根治)
- 建议补充 `Unregister<T>()` 方法,便于单元测试隔离
---
### 1.3 ScriptableObject 事件频道系统Event Channel SO✅ 行业标准
基于 Unity Open Projects 推广的 SO 事件频道模式:
```csharp
public abstract class BaseEventChannelSO<T> : ScriptableObject
{
public EventSubscription Subscribe(Action<T> callback) { ... } // 返回可 Dispose 句柄
}
```
**优势**
- 发布者/订阅者完全解耦,不需要互相持有引用
- `EventSubscription` + `CompositeDisposable` 构成 Rx-like 生命周期管理,防内存泄漏
- `EventBusMonitor` 在 Editor 下记录每次调用频道名、payload、监听器数量、帧号调试体验优异
- SO 作为 Inspector 资产,便于在 Prefab 中任意组合引用,无需代码修改
**不足**
- 缺少 **事件频道查找表**25+ 个 SO 频道资产散落在 `Assets/Data/` 下,若多人协作或频道名称不统一,容易创建重复频道。`EventChannelRegistry` 已有雏形,建议强制所有频道注册其中
---
### 1.4 游戏状态机GameStateMachine✅ 稳健
```csharp
public bool TransitionTo(GameStateId nextId, out string error)
{
if (!_current.ValidNextStates.Contains(nextId)) { error = ...; return false; }
_current?.OnExit(nextId);
_current = next;
_current.OnEnter(prev);
...
}
```
**优势**
- 转换合法性在状态机层面校验(`ValidNextStates`),而非散落在各处的 if-else
- 状态对象不继承 `MonoBehaviour`,纯 C# 类,可单元测试
- `[DefaultExecutionOrder(-1000)]` 确保 GameManager 最先 Awake
**不足**
- `IGameState.ValidNextStates` 如果用 `HashSet<GameStateId>` 而非 `IEnumerable<GameStateId>` 可避免 `Contains()` 的 O(n) 查找(当前状态数少影响不大,但属于最佳实践缺失)
---
### 1.5 玩家状态机Player FSM 有结构性问题
`PlayerController` 声明了 16 个具体状态字段:
```csharp
private IdleState _idleState;
private RunState _runState;
private JumpState _jumpState;
// ... 共 16 个
```
**优势**
- 状态为纯 POCOPlain Old C# Object无 MonoBehaviour 开销
- `PlayerStateBase` 持有 Controller 引用并暴露属性,状态类代码简洁
**不足(商业级对比)**
- **强耦合**:新增状态需修改 `PlayerController`违反开闭原则。Dead Cells、Celeste 等同类项目通常用 `Dictionary<Type, PlayerStateBase>` 或枚举映射表管理状态,`PlayerController` 只需调用 `_states[StateType.Idle]`
- **状态切换 API 散落**`Owner.TryTransitionState(Owner.IdleState)``Owner.IdleState` 是公开属性,但这要求 `PlayerController` 对外暴露所有状态实例,破坏封装
- **HitBox 时间点硬编码在状态类中**`AttackState``events.Add(0.3f, ...)``events.Add(0.6f, ...)` 应配置在 `PlayerAnimationConfigSO` 而非状态代码中
---
### 1.6 伤害流水线 ✅ 设计完善
```
HitBox.OnTriggerEnter2D → DamageInfo.From(SO) → HurtBox.ReceiveDamage
→ [1] 无敌帧 → [2] 弹反 → [3] 霸体 → [4] 护盾 → [5] 防御减免 → [6] HP 扣除
→ [7] 状态效果 → [8] 事件广播
```
8 步流水线文档清晰,`DamageInfo` struct 使用 Builder 模式和零 GC 工厂方法 `From(SO)`,接口驱动(`IDamageable``IShieldable``IPoiseSource``IStatusEffectable`)使各步骤可独立替换。
---
### 1.7 单例模式滥用 ⚠️ 需关注
全项目存在以下单例:
| 类名 | 必要性评估 |
|------|-----------|
| `GameManager.Instance` | 合理(全局生命周期协调者)|
| `SaveManager.Instance` | 合理(跨场景持久)|
| `GlobalObjectPool.Instance` | 合理(全局池)|
| `ClashResolver.Instance` | ⚠️ 可通过 ServiceLocator 消除 |
| `ProjectileManager.Instance` | ⚠️ 可通过 ServiceLocator 消除 |
| `DifficultyManager.Instance` | ⚠️ 可通过 ServiceLocator 消除 |
商业项目通常将"功能性单例"注册到 ServiceLocator仅保留 2-3 个真正的全局单例。当前 6 个单例增加了测试和多场景管理的难度。
---
## 二、性能
### 2.1 对象池 ✅ 专业实现
```csharp
public class GlobalObjectPool : MonoBehaviour
{
private readonly Dictionary<string, Queue<PooledObject>> _pools = new();
private readonly Dictionary<string, List<PooledObject>> _alive = new();
...
public T Spawn<T>(string key, Vector3 position, Quaternion rotation) where T : Component
}
```
**优势**
- 双数据结构(空闲队列 + 活跃列表)支持上限控制和活跃对象回收
- 同时提供 `IEnumerator WarmupCoroutine()``async Task WarmupAsync()`,适应 MonoBehaviour 和纯 C# 两种调用场景
- Addressables 驱动预热,支持异步加载资源
**不足**
- `GetComponentCached<T>()` 是否做了组件缓存?若每次 Spawn 都 `GetComponent`,频繁 Spawn 时仍有 GC 压力。建议在 `PooledObject.Awake` 缓存各常用组件接口
---
### 2.2 批量视线检测BatchLOSSystem✅ 优化到位
```csharp
// 每 FixedUpdate 只检测部分敌人Round-Robin 分帧)
int count = Mathf.Min(_maxRequestersPerFrame, _requesters.Count);
for (int i = 0; i < count; i++)
{
int idx = (_currentOffset + i) % _requesters.Count;
Physics2D.Raycast(...);
}
_currentOffset = (_currentOffset + count) % ...;
```
**优势**
- 分帧批处理,避免同帧对 20+ 敌人全部执行 Raycast
- 接口驱动(`ILOSRequester`),与 `EnemyBase` 解耦
**不足**
- `_requesters.RemoveAt(idx)` 是 O(n) 操作,敌人频繁死亡/重生时有性能隐患。商业实现通常用标记-清除null 标记 + 批量清理)代替直接移除
---
### 2.3 DamageInfo 零 GC ✅
```csharp
// 热路径struct 工厂,无堆分配
var info = DamageInfo.From(_currentSource);
info.KnockbackDirection = knockDir;
info.KnockbackForce = _currentSource.KnockbackForce;
```
`DamageInfo` 设计为非 readonly struct便于 Builder 就地写入),同时提供 `From(SO)` 零 GC 工厂路径,在命中密集场景(群战)中显著减少 GC 压力,是商业级实现水准。
---
### 2.4 潜在性能隐患
| 位置 | 问题 | 建议 |
|------|------|------|
| `HitBox.cs` | `_hitCooldownTimers: Dictionary<Collider2D, float>` 每帧 `Mathf.Max` 遍历,高频场景有 GC | 改用固定大小数组 + 索引映射 |
| `GameServiceRegistrar.EnsureSingleAudioListener()` | `FindObjectsOfType<AudioListener>` 每次场景加载调用 | 可接受(非热路径),已限制场景加载时调用 |
| `AttackState.PlayAttackClip()` | 每次进入攻击状态都 `state.Events(this)` 注册 OnEnd 和帧事件 | Animancer 内部有缓存,影响有限,但帧事件时间点应提取到 SO |
| `EnemyQuotaManager` | 未见实现细节,需确认波次结算时无 LINQ 热路径 | - |
---
## 三、可扩展性
### 3.1 接口驱动设计 ✅ 充分
| 接口 | 用途 | 实现类 |
|------|------|--------|
| `IDamageable` | 可受击 | `PlayerController`, `EnemyBase` |
| `IShieldable` | 护盾拦截 | `ShieldComponent` |
| `IPoiseSource` | 霸体来源 | `PlayerController`, `EnemyBase` |
| `IBreakable` | 可破坏机关 | 场景对象 |
| `IStatusEffectable` | 状态效果 | `StatusEffectManager` |
| `IPathAgent` | 导航代理 | `EnemyNavAgent`Navigation 程序集)|
| `ISaveStorage` | 存档存储后端 | `LocalFileStorage`(可替换为云存储)|
| `ILOSRequester` | 视线检测请求者 | `EnemyBase` 实现 |
**跨程序集依赖反转做得好**`HurtBox`Combat 程序集)通过 `IStatusEffectable` 接口调用 `StatusEffectManager`StatusEffects 程序集),避免反向依赖。
---
### 3.2 ScriptableObject 数据资产驱动 ✅
- `DamageSourceSO`:伤害源(伤害值、类型、标记)独立资产,策划可直接编辑
- `PlayerMovementConfigSO`:所有移动参数集中配置
- `EnemyStatsSO`:敌人属性数据分离
- `BossSkillSO` + `AttackPatternSO`Boss 技能和攻击模式纯数据化
- `DifficultyScalerSO`:难度缩放系数可独立调节
这是 Hollow Knight 同类项目的标准实践,大幅降低策划-程序沟通成本。
---
### 3.3 存档迁移SaveMigrator✅ 生产级
```csharp
case "1.0": data = MigrateFrom1_0(data); goto case "1.1";
case "1.1": data = MigrateFrom1_1(data); goto case "2.0";
case "2.0": data = MigrateFrom2_0(data); goto case "2.1";
```
版本链式迁移fall-through支持从任意旧版本升级到最新版加上 checksum 校验,是 AA 级游戏存档系统的标准实现。
---
### 3.4 扩展瓶颈
**1. 玩家状态扩展成本高**
新增一个玩家状态(如游泳攻击状态)需要:
1. 新建 `SwimAttackState.cs`
2.`PlayerController` 中声明字段 `private SwimAttackState _swimAttackState;`
3.`PlayerController.Awake()` 中初始化
4. 在相关状态的 `OnStateUpdate()` 中添加跳转判断
对比 Dead Cells 等商业项目的状态字典方案,每次新增需修改 Controller 核心类,违反开闭原则。
**2. Boss 技能扩展**
`BossSkillSO` + `AttackPatternSO` + `SkillSequenceSO` 组合提供了良好的数据驱动扩展能力,但 `BossSkillExecutor` 的具体执行逻辑需要查看是否支持新技能类型不修改代码。
---
## 四、编辑器友好性
### 4.1 EventBusMonitor ✅ 出色
```csharp
#if UNITY_EDITOR
EventBusMonitor.Record(name, value?.ToString() ?? "null",
OnEventRaised?.GetInvocationList().Length ?? 0,
Time.frameCount);
#endif
```
每次 EventChannel 触发均记录频道名、payload 字符串表示、当前监听器数量、帧号、时间戳。配合自定义 EditorWindow 可实现完整事件流可视化,是商业项目必备的调试工具。`#if UNITY_EDITOR` 包裹确保零 Release 开销。
---
### 4.2 Inspector 标注 ✅
- `[Header(...)]` 分组清晰:`PlayerController` 中 "核心组件"、"配置"、"战斗"、"事件频道" 分组一目了然
- `[DefaultExecutionOrder]` 在 5 个关键类上正确标注,执行顺序有保证
- `[RequireComponent(typeof(...))]``HitBox``HurtBox``PlayerMovement` 等关键组件上使用,防止配置错误
- `[Multiline]` 用于 EventChannel 的 description 字段,方便编辑器中多行文字说明
---
### 4.3 `IValidatable` 接口 ⚠️ 未完全落地
`Core/Validation/IValidatable.cs` 定义了验证接口,但未见对应的 Editor 自动扫描器(如 `OnValidate()` 批量调用),导致验证逻辑无法在编辑器保存时自动触发。建议补充:
```csharp
// Editor/ValidatorEditor.cs
[MenuItem("BaseGames/Validate All ScriptableObjects")]
static void ValidateAll()
{
foreach (var so in Resources.FindObjectsOfTypeAll<ScriptableObject>())
if (so is IValidatable v) v.Validate();
}
```
---
### 4.4 SO 资产菜单 ✅
关键数据类均有 `[CreateAssetMenu]`,策划可直接右键创建,无需找程序,符合现代 Unity 工作流。
---
## 五、使用便利性API 设计)
### 5.1 EventChannel 订阅 API ✅ 流畅
```csharp
// 推荐用法:链式订阅,自动生命周期管理
_onPlayerDied.Subscribe(HandlePlayerDied).AddTo(_subscriptions);
// OnDisable 一行清理
_subscriptions.Clear();
```
`EventSubscription.AddTo()` 扩展方法是良好的 Fluent API 设计,使用体验接近 UniRx。
---
### 5.2 对象池 API ✅
```csharp
// 泛型获取,类型安全
var proj = GlobalObjectPool.Instance.Spawn<LinearProjectile>("Proj_Arrow", pos, rot);
```
API 简洁,类型安全,无需 `GetComponent` 手动转型。
---
### 5.3 存档 API ✅
```csharp
// async/await 风格UI 层友好
await SaveManager.Instance.SaveAsync(slot: 0);
bool ok = await SaveManager.Instance.LoadAsync(slot: 0);
```
现代 async/await 模式,配合存档指示器事件频道,完整覆盖 UI 反馈需求。
---
### 5.4 InputReaderSO 输入 API ⚠️ 轻微不足
```csharp
// 事件驱动(✅ 推荐用法)
_inputReader.AttackEvent += OnAttack;
// 轮询(✅ 也支持)
Vector2 move = _inputReader.MoveInput;
```
**不足**`InputReaderSO` 是 ScriptableObject跨场景加载时 `OnEnable/OnDisable` 行为依赖 Unity 的 SO 生命周期,容易在热重载或 Play-mode 重新进入时出现"Map must be contained in state"错误(代码注释中已提及,但解决方案(`EnsureInitialized` + Reset属于防御性修复而非根治。建议将 Input 初始化移至专用 MonoBehaviour 组件(如现有的 `InputReaderBootstrap.cs`)中管理。
---
### 5.5 硬编码问题汇总
| 位置 | 硬编码内容 | 建议 |
|------|-----------|------|
| `HitBox.cs:42-43` | `PlayerHitBoxLayer = 13`, `EnemyHitBoxLayer = 16` | 改用 `LayerMask` 字段或 `Physics2D.GetLayerCollisionMask` |
| `AttackState.cs:38-40` | `events.Add(0.3f, ...)` 命中盒激活时间 | 移至 `PlayerAnimationConfigSO``HitBoxActiveStart/End` 字段 |
| `PlayerMovement.cs:58` | `CoyoteTime` 默认值 `0.12f` 内联 fallback | SO 必填,移除内联 fallback 以强制配置 |
| `SaveManager.cs:13` | `QuickSaveSlot = 98` | 移至 `GlobalSettingsSO` |
---
## 六、与商业项目对比
### 对比基准Hollow Knight / Dead Cells 技术架构(公开分析资料)
| 对比项 | 本项目 | 商业基准 | 差距 |
|--------|--------|---------|------|
| 程序集隔离 | 25 个 asmdef | 通常 15-30 个 | ✅ 持平 |
| 事件系统 | SO EventChannel + CompositeDisposable | 通常 SO Channel 或自研消息总线 | ✅ 持平 |
| 玩家状态机 | POCO 状态 + Controller 字段 | 通常状态字典/工厂,更低耦合 | ⚠️ 有差距 |
| 对象池 | Addressables + Queue支持上限 | 商业项目通常更完整(类型缓存、统计面板) | ✅ 基本持平 |
| 存档系统 | JSON + checksum + 版本迁移链 | ✅ 已达商业标准 | ✅ 持平 |
| 伤害流水线 | 8 步,接口驱动 | 商业项目通常 6-10 步 | ✅ 持平 |
| 难度系统 | SO Scaler + ISaveable | 商业标准实现 | ✅ 持平 |
| 输入系统 | Unity Input System + Buffer | 商业标准实现 | ✅ 持平 |
| 单例数量 | 6 个 | 建议 ≤ 3 个,其余用 ServiceLocator | ⚠️ 偏多 |
| 测试支持 | ServiceLocator 支持 Mock但无测试用例 | 商业项目通常有核心系统单元测试 | 🔴 缺失 |
| CI/自动验证 | 无 | 商业项目通常有 | 🔴 缺失 |
---
## 七、优先级改进建议
### 🔴 高优先级(影响开发效率或潜在 Bug
1. **PlayerController 状态管理重构**
将 16 个状态字段改为 `Dictionary<System.Type, PlayerStateBase>` + 工厂方法,新增状态无需修改 Controller
2. **AttackState 帧事件时间点提取**
`events.Add(0.3f, ...)` 移至 `PlayerAnimationConfigSO`,支持策划在 Inspector 调节命中盒窗口而无需修改代码
3. **HitBox 层级硬编码消除**
`PlayerHitBoxLayer = 13` 改为 `[SerializeField] LayerMask _rivalHitBoxMask`,配置驱动,层级重排时不破坏逻辑
### 🟠 中优先级(提升可维护性)
4. **ServiceLocator 替换功能性单例**
`ClashResolver``ProjectileManager``DifficultyManager` 注册到 `ServiceLocator`,消除直接 `Instance` 引用
5. **EventChannelRegistry 强制注册**
要求所有 EventChannel SO 在 `GameServiceRegistrar.Awake` 时注册到 `IEventChannelRegistry`,防止孤立频道
6. **核心类迁移 DamageInfo / HitInfo**
`Core/Events/` 移至 `Combat/`,保持程序集领域边界清晰
### 🟡 低优先级(质量提升)
7. **补充 IValidatable Editor 扫描器**
批量验证 SO 配置,减少运行时 `null` 警告
8. **BatchLOSSystem 移除优化**
改用标记清除替代 `RemoveAt`,消除 O(n) 开销
9. **补充核心系统单元测试**
`GameStateMachine``DamageInfo.Builder``SaveMigrator` 逻辑简单,非常适合作为第一批测试用例
---
## 八、总结
本项目代码质量在国内独立游戏中属于**偏上水准**核心架构模式SO 事件系统、程序集隔离、接口驱动伤害流水线、存档迁移)均达到或接近商业标准。
主要差距集中在两类问题:
1. **结构性**:玩家状态机扩展成本偏高,单例过多
2. **规范性**:少量硬编码、未落地的验证基础设施、缺少自动化测试
这两类问题不影响当前功能运作,但随项目规模增大(尤其玩家状态和 Boss 数量增加)会造成明显的维护负担。建议在 Phase 3-4 内容开发前完成高优先级改进。