多轮审查和修复
This commit is contained in:
606
Docs/Review/DeepDive_2026.md
Normal file
606
Docs/Review/DeepDive_2026.md
Normal file
@@ -0,0 +1,606 @@
|
||||
# zeling_v2 深度代码评审 2026
|
||||
|
||||
> **评审日期**：2026-01
|
||||
> **评审范围**：`Assets/Scripts/` 全部 ~415 个 `.cs` 文件
|
||||
> **评审标准**：以同等体量商业 2D 动作游戏（Hollow Knight / Celeste / Hades）代码质量为基准
|
||||
> **代码终态**：本轮评审基于所有 P1-P3 修复全部落地后的最终版本
|
||||
|
||||
---
|
||||
|
||||
## 评分总览
|
||||
|
||||
| 维度 | 得分（/10） | 说明 |
|
||||
|------|------------|------|
|
||||
| 架构设计 | **8.2** | 程序集隔离 + ServiceLocator + SO Event Channel 构成优秀骨架，少量单例混用 |
|
||||
| 性能 | **8.0** | 热路径零分配设计扎实，BatchLOS 时间切片；小处存在可优化空间 |
|
||||
| 可扩展性 | **8.3** | 数据驱动策略模式普遍应用，工厂字典 + 版本迁移链完备 |
|
||||
| 编辑器友好 | **8.2** | 工具链完整（监控/验证/可视化），Post-fix 后类型化，个别窗口健壮性待补 |
|
||||
| 使用便利性（DX） | **8.2** | API 命名清晰，订阅生命周期管理完善，混用模式尚存，部分接口语义有歧义 |
|
||||
| **综合** | **8.2** | 同等体量独立游戏上游水准，离头部商业游戏代码约 0.5–1.0 分距离 |
|
||||
|
||||
---
|
||||
|
||||
## 一、架构设计
|
||||
|
||||
### 1.1 程序集隔离（Assembly Definitions）
|
||||
|
||||
25 个 `.asmdef` 文件将模块划分为独立编译单元，强制单向依赖：
|
||||
|
||||
```
|
||||
Core → Input → Player → Combat → Enemies → World → ...
|
||||
```
|
||||
|
||||
- **无循环依赖**：`BaseGames.Parry` 不引用 `BaseGames.Combat`，`ConsumeParry()` 签名无 `DamageInfo` 参数——这种取舍有意为之，体现了架构约束的一致贯彻。
|
||||
- **接口切面**：`ILOSRequester`、`IDamageable`、`IPoiseSource`、`IStatusEffectable`、`IObjectPoolService`、`IPlatformService` 等接口均定义在依赖链上游，让下游实现以干净方式向上暴露能力。
|
||||
- **条件编译护栏**：`#if GRAPH_DESIGNER`、`#if STEAMWORKS_NET`、`#if UNITY_EDITOR` 三类编译守卫严格隔离平台/工具代码，防止运行时包体污染。
|
||||
|
||||
**评价**：程序集设计达到商业中等水准。大型工作室（如 Team Cherry）同样采用类似分层策略。
|
||||
|
||||
---
|
||||
|
||||
### 1.2 服务定位器（ServiceLocator）
|
||||
|
||||
```csharp
|
||||
// 静态字典，Unity 主线程单线程访问，无锁竞争
|
||||
private static readonly Dictionary<Type, object> _services = new();
|
||||
|
||||
public static T Get<T>() // 缺失时抛出异常，快速失败
|
||||
public static T GetOrDefault<T>() // 缺失时返回 null，防御性访问
|
||||
public static void RegisterIfAbsent<T>(...) // 幂等注册
|
||||
|
||||
#if UNITY_EDITOR
|
||||
public static void OverrideForTest<T>(...) // 测试注入点
|
||||
#endif
|
||||
```
|
||||
|
||||
- **设计优点**：单责任（查找/注册），`Get<T>()` 快速失败语义明确，`GetOrDefault<T>()` 防御性访问分开，Editor 测试注入不污染运行时。
|
||||
- **局限**：静态状态在 Domain Reload 后需要手动清理（Unity 在 `[InitializeOnLoad]` 中已有对应处理）；构造图对新人不可见。
|
||||
|
||||
**对比**：Hades 的引擎使用类似的 Service Locator（而非 DI 容器），同规模项目此方案已足够。
|
||||
|
||||
---
|
||||
|
||||
### 1.3 SO Event Channel 系统
|
||||
|
||||
```csharp
|
||||
// BaseEventChannelSO<T>
|
||||
private event Action<T> _action; // 私有 backing field，防止外部 = 覆盖
|
||||
public event Action<T> Action { add => _action += value; remove => _action -= value; }
|
||||
|
||||
// 订阅返回 IDisposable，支持 CompositeDisposable.AddTo() 生命周期绑定
|
||||
public EventSubscription Subscribe(Action<T> handler) { ... }
|
||||
```
|
||||
|
||||
| 特性 | 评估 |
|
||||
|------|------|
|
||||
| 私有 backing field | ✅ 防外部 `=` 覆盖，只能 `+=/-=` |
|
||||
| EventSubscription（readonly struct） | ✅ 零分配，IDisposable 自动反订阅 |
|
||||
| CompositeDisposable.AddTo() | ✅ 生命周期与 MonoBehaviour 绑定，无泄漏 |
|
||||
| EventBusMonitor 256 条环形记录 | ✅ 运行时事件流可观测 |
|
||||
| Editor 订阅者计数 | ✅ 0 订阅者时红色高亮警告 |
|
||||
|
||||
**最强设计点**：`EventSubscription` 作为 `readonly struct` 实现 `IDisposable`，避免了大量项目中常见的"忘记取消订阅"内存泄漏问题，与 UniRx/R3 的 Disposable 模式对齐。
|
||||
|
||||
---
|
||||
|
||||
### 1.4 状态机体系
|
||||
|
||||
**玩家状态机**（`PlayerStateBase` + `PlayerController`）
|
||||
- 纯 C# 状态对象（无 MonoBehaviour 开销），构造器注入依赖，`ValidTransitions` 白名单 `#if UNITY_EDITOR` 校验。
|
||||
- `Dictionary<Type, PlayerStateBase> _states` O(1) 查找，无反射。
|
||||
|
||||
**游戏状态机**（`GameStateMachine`）
|
||||
- 纯 C# 类（不挂 MonoBehaviour），`Dictionary<GameStateId, IGameState>` + `ValidNextStates` 白名单，`TransitionTo()` 失败返回 `error` 字符串而非抛异常。
|
||||
|
||||
**敌人状态机**（`EnemyBase._stateObjs`）
|
||||
- `Dictionary<EnemyStateType, IEnemyState>` POCO 实现，状态由子类在 Awake 填充，基类不假定具体状态集。
|
||||
|
||||
**弹反状态机**（`ParrySystem`）
|
||||
- 枚举 `ParryPhase { Inactive, Startup, Active, EndLag, CounterWindow }` 清晰定义相变，`Update` 驱动计时，`IsParrying`/`IsInCounterWindow` 布尔属性暴露只读状态。
|
||||
|
||||
**潜在问题**：
|
||||
- `GameStateMachine.TransitionTo` 中 `_current.ValidNextStates.Contains()` 若 `ValidNextStates` 为 `IEnumerable`（线性查找），在频繁转换时存在微小 O(n) 开销。建议内部改用 `HashSet<GameStateId>`。
|
||||
|
||||
---
|
||||
|
||||
### 1.5 存档系统
|
||||
|
||||
```
|
||||
SaveManager（SemaphoreSlim 并发锁）
|
||||
├── ISaveable（注册接口）
|
||||
├── ISaveStorage（LocalFileStorage 实现）
|
||||
├── SaveMigrator（goto 版本链 1.0→1.1→2.0→2.1）
|
||||
└── SaveData（Newtonsoft.Json 序列化）
|
||||
```
|
||||
|
||||
- `SemaphoreSlim(1,1)` 防止并发写入损坏文件。
|
||||
- Checksum 两步计算（先 null→序列化→计算→填入→再序列化）正确且清晰注释。
|
||||
- `SaveMigrator` 的 `goto case` 是 C# 语言规范中唯一正确的 switch-fallthrough 写法，并非 bad practice。
|
||||
|
||||
**混用模式问题（-0.2 分）**：`SaveManager.Instance`、`VFXPool.Instance`、`GlobalObjectPool.Instance` 为传统单例，而其他服务均通过 `ServiceLocator` 访问。其中 `GlobalObjectPool` 在 Awake 同时注册 `ServiceLocator.Register<IObjectPoolService>(this)`，形成双重访问路径，容易造成混淆。
|
||||
|
||||
---
|
||||
|
||||
### 1.6 架构设计待改进项
|
||||
|
||||
| 问题 | 影响 | 建议 |
|
||||
|------|------|------|
|
||||
| `SaveManager.Instance` 未接入 ServiceLocator | 模式不一致，测试困难 | `ISaveManager` 接口 + ServiceLocator 注册 |
|
||||
| `AnalyticsManager` 无 namespace 声明 | 全局命名空间污染 | 添加 `namespace BaseGames.Support.Analytics` |
|
||||
| `PlayerController.GetCurrentPoiseLevel()` 硬返回 `PoiseLevel.None` | 误导性 API，调用者以为玩家有霸体 | 注释说明或移除方法，改为常量字段 |
|
||||
| `IGameState.ValidNextStates` 为 `IEnumerable` | 状态转换 O(n) | 改为 `IReadOnlySet<GameStateId>` 实现 |
|
||||
|
||||
---
|
||||
|
||||
## 二、性能
|
||||
|
||||
### 2.1 热路径零分配设计
|
||||
|
||||
| 类型 | 技术 | 效果 |
|
||||
|------|------|------|
|
||||
| `DamageInfo` | `struct` + `From()` 工厂 | 无堆分配 |
|
||||
| `EventSubscription` | `readonly struct IDisposable` | 无堆分配 |
|
||||
| `HurtBox` 8 步管线 | 纯 struct/值类型操作 | 每帧命中无 GC 压力 |
|
||||
| `HitBox` 命中去重 | `HashSet<Collider2D>` | O(1) per check |
|
||||
| `MaterialPropertyBlock` | StatusEffectManager 渲染 | 不产生材质实例 |
|
||||
| `Newtonsoft.Json Formatting.None` | SaveManager | 减小序列化字符串体积 |
|
||||
| `ValidTransitions` 白名单 | `#if UNITY_EDITOR` 只在 Editor 执行 | 运行时无额外开销 |
|
||||
|
||||
---
|
||||
|
||||
### 2.2 BatchLOSSystem 时间切片
|
||||
|
||||
```csharp
|
||||
[DefaultExecutionOrder(-200)] // 保证在 EnemyBase.FixedUpdate 之前执行
|
||||
private const int _maxRequestersPerFrame = 8;
|
||||
|
||||
// 均匀旋转偏移，避免每帧处理同一批请求
|
||||
private int _offset;
|
||||
for (int i = 0; i < _maxRequestersPerFrame; i++)
|
||||
{
|
||||
int idx = (_offset + i) % _requesters.Count;
|
||||
// ... raycast + callback
|
||||
}
|
||||
_offset = (_offset + _maxRequestersPerFrame) % _requesters.Count;
|
||||
```
|
||||
|
||||
**设计优点**：无论敌人数量多少，每帧固定 8 次 Raycast，时间复杂度 O(1) per frame。通过轮转偏移保证每个请求者以均匀频率得到更新。
|
||||
|
||||
**待优化（-0.15 分）**：
|
||||
- `_requesters` 使用 `List<ILOSRequester>`，`Register` 时内部 `Contains()` 为 O(n)。当场景内存在 >30 个敌人时，批量注册阶段（关卡加载）会产生 O(n²) 开销。建议改用 `HashSet<ILOSRequester>` 或 `(List + HashSet)` 双结构。
|
||||
|
||||
---
|
||||
|
||||
### 2.3 VFXPool（P3-10 修复后）
|
||||
|
||||
```
|
||||
Play(vfxRef, position, ...)
|
||||
├── TryDequeue() 命中 → PlayImmediate（同帧播放，无 Addressables 等待）
|
||||
└── 未命中 → PlayLoadAsync（异步加载 + 实例化）
|
||||
```
|
||||
|
||||
修复前每次 `Play()` 即使池中已有实例也要经过一帧协程等待。修复后池命中路径 **0 帧延迟**，与 Celeste 的预热粒子池策略一致。
|
||||
|
||||
---
|
||||
|
||||
### 2.4 GlobalObjectPool
|
||||
|
||||
- `WarmupAsync()` 预热，避免运行时 Addressables.InstantiateAsync 延迟。
|
||||
- `Dictionary<string, Queue<PooledObject>>` 池 + `Dictionary<string, LinkedList<PooledObject>>` 活跃链表：O(1) 入队/出队 + O(1) 强制回收（LinkedList.Remove 为 O(1) 已知节点）。
|
||||
- `MaxCount > 0` 限制池 + 活跃对象总量，防止无限扩张。
|
||||
|
||||
---
|
||||
|
||||
### 2.5 性能待改进项
|
||||
|
||||
| 问题 | 位置 | 严重度 | 建议 |
|
||||
|------|------|--------|------|
|
||||
| `BatchLOSSystem._requesters.Contains()` O(n) | `Register()` | 中 | 改用 `HashSet<ILOSRequester>` |
|
||||
| `EquipmentManager.UsedNotches` 每次调用 LINQ `Sum()` | 属性 getter | 低 | 维护 `_usedNotches` 缓存字段，装备/卸下时增减 |
|
||||
| `AnalyticsManager.Track()` 创建 `new Dictionary<string,object>` | 每次调用 | 低（非热路径） | 在 Gameplay 密集调用点使用静态预分配 |
|
||||
| `DamageInfo` 非 `readonly struct` | `DamageInfo.cs` | 低 | 标记为 `readonly struct`，Builder 内部操作局部变量 |
|
||||
| `EventBusMonitor.Queue<EventRecord>` struct 装箱 | Editor only | 极低 | 换 `EventRecord[]` 环形数组 + int head/tail |
|
||||
| `ClashResolver` XOR key 碰撞 | `ResolveClash()` | 极低 | 用 `(int, int)` 对元组替代 XOR |
|
||||
|
||||
---
|
||||
|
||||
## 三、可扩展性
|
||||
|
||||
### 3.1 数据驱动架构（ScriptableObject）
|
||||
|
||||
以下系统全面采用 SO 数据驱动：
|
||||
|
||||
| 系统 | SO 类型 | 可配置项 |
|
||||
|------|---------|----------|
|
||||
| 护符 | `CharmSO` + `ICharmEffect[]` | 策略模式，效果任意组合 |
|
||||
| 技能 | `FormSkillSO` + `SkillSlotOverride` | 护符可覆盖技能槽 |
|
||||
| Boss 技能 | `BossSkillSO` + `SkillSequenceSO` | Windup/Active/Recovery 三段配置 |
|
||||
| 伤害源 | `DamageSourceSO` | 直接 `DamageInfo.From(so)` 零分配构建 |
|
||||
| 弹反配置 | `ParryConfigSO` | 前摇/后摇/反击窗口毫秒级可调 |
|
||||
| 装备 | `EquipmentConfigSO` | 初始 Notch 数 |
|
||||
| 世界状态 | `WorldStateRegistry` (SO) | `OnEnable` 自动清理，Editor 安全 |
|
||||
|
||||
**`ICharmEffect` 策略模式**是可扩展性最强的设计：新护符效果只需实现接口，无需修改任何管理器代码，完全符合开闭原则。
|
||||
|
||||
---
|
||||
|
||||
### 3.2 StatusEffect 工厂字典（P2-5 修复后）
|
||||
|
||||
```csharp
|
||||
// 修复前：静态 switch，新效果必须修改 StatusEffectManager 源码
|
||||
// 修复后：
|
||||
private readonly Dictionary<DamageType, Func<StatusEffect>> _effectFactories = new();
|
||||
|
||||
public void RegisterEffectFactory(DamageType type, Func<StatusEffect> factory)
|
||||
=> _effectFactories[type] = factory;
|
||||
|
||||
// Boss 模块可在运行时注册自定义效果
|
||||
StatusEffectManager.RegisterEffectFactory(DamageType.Dark, () => new DarkCurseEffect());
|
||||
```
|
||||
|
||||
对外扩展点与运行时动态注册并存，是 Hades 中 Boon 效果系统类似的做法。
|
||||
|
||||
---
|
||||
|
||||
### 3.3 SaveMigrator 版本迁移链
|
||||
|
||||
```csharp
|
||||
switch (data.Meta.Version)
|
||||
{
|
||||
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";
|
||||
case "2.1": break;
|
||||
}
|
||||
```
|
||||
|
||||
- 新版本只需添加一个新的 `case` + `goto`，旧版本代码不变。
|
||||
- 每个迁移函数独立，测试友好（传入 `SaveData` 验证输出）。
|
||||
- `null` 合并运算符 `??=` 处理旧版本缺失字段，不影响新版本正常路径。
|
||||
|
||||
**局限**：版本字符串 `"1.0"` 为 magic string，若版本标识改为 `int`/`enum` 可减少拼写错误风险。
|
||||
|
||||
---
|
||||
|
||||
### 3.4 平台抽象层
|
||||
|
||||
```csharp
|
||||
// IPlatformService 接口
|
||||
// SteamPlatformService：#if UNITY_STANDALONE && STEAMWORKS_NET
|
||||
// ConsolePlatformService（预留）
|
||||
// MockPlatformService（测试用）
|
||||
```
|
||||
|
||||
多平台切换不需要修改任何业务代码。与 Hades / Cuphead 多平台发布策略一致。
|
||||
|
||||
---
|
||||
|
||||
### 3.5 WorldStateRegistry 泛化 API
|
||||
|
||||
```csharp
|
||||
// 泛化版本（直接使用）
|
||||
public bool IsMarked(WorldObjectCategory category, string id)
|
||||
public void Mark(WorldObjectCategory category, string id)
|
||||
|
||||
// 具名别名（向后兼容）
|
||||
public bool IsCollected(string id) => IsMarked(WorldObjectCategory.Collectible, id);
|
||||
public void MarkDestroyed(string id) => Mark(WorldObjectCategory.Destroyed, id);
|
||||
```
|
||||
|
||||
新类别只需在枚举添加值，无需修改逻辑层，同时保留具名 API 的可读性。`OnStateChanged` 事件允许 UI/测试代码响应式订阅，不耦合到具体业务逻辑。
|
||||
|
||||
---
|
||||
|
||||
### 3.6 可扩展性待改进项
|
||||
|
||||
| 问题 | 影响 | 建议 |
|
||||
|------|------|------|
|
||||
| `SkillManager` 技能槽硬编码为字符串 `"SoulSkill"/"SpiritSkill1"/"SpiritSkill2"` | 新形态需修改多处字符串 | 抽象为 `SkillSlotId enum` 或 `const string` 集中管理 |
|
||||
| `EquipmentManager._collected.Contains()` 为 O(n) | 100+ 护符时 | 改用 `HashSet<CharmSO> _collectedSet` |
|
||||
| `SaveMigrator` 版本为 magic string | 拼写错误难察觉 | 改为 `Version` 类或 int 常量 |
|
||||
| `PlayerController.GetCurrentPoiseLevel()` 始终返回 `None` | 玩家霸体无法实现 | 实现基于护符/状态的动态计算 |
|
||||
|
||||
---
|
||||
|
||||
## 四、编辑器友好性
|
||||
|
||||
### 4.1 工具链全景
|
||||
|
||||
```
|
||||
BaseGames/Tools/
|
||||
├── Event Bus Monitor Ctrl+Shift+E — 运行时事件流监控
|
||||
├── Boss Skill Sequence Viewer — 甘特图可视化 Boss 技能时序
|
||||
├── Validate Address Keys — Addressables key 一致性检查
|
||||
└── SOValidationRunner (Build Hook) — ScriptableObject 完整性验证
|
||||
```
|
||||
|
||||
这四个工具覆盖了**运行时调试**、**设计验证**、**构建前检查**三个阶段，形成完整的质量保障链。
|
||||
|
||||
---
|
||||
|
||||
### 4.2 EventBusMonitorWindow
|
||||
|
||||
```
|
||||
[Time] [Frame] [Channel] [Payload] [Subs]
|
||||
0.234 143 OnPlayerTakeDamage {amount:12.0} 3
|
||||
0.251 144 OnEnemyDied {id:"Slug_01"} 2
|
||||
0.267 145 OnHealPickup ← 0 subs 0 ← 红色警告行
|
||||
```
|
||||
|
||||
- **Filter**：实时文本过滤，`IndexOf` 大小写不敏感。
|
||||
- **Pause Capture**：保留历史快照不被新事件覆盖。
|
||||
- **Auto Scroll**：`_scroll.y = float.MaxValue` 强制滚底。
|
||||
- **0 订阅者红色高亮**：立即暴露频道配错或漏连的问题。
|
||||
|
||||
**唯一缺陷**：`EventBusMonitor` 后端使用 `Queue<EventRecord>` 而非固定大小数组，每次超出 256 条时 `Dequeue()` 有轻微 GC（Editor only，可接受）。
|
||||
|
||||
---
|
||||
|
||||
### 4.3 BossSkillSequenceWindow
|
||||
|
||||
甘特图实时渲染 Boss 技能相位：
|
||||
|
||||
| 相位 | 颜色 | 含义 |
|
||||
|------|------|------|
|
||||
| Windup | 黄色 | 前摇 |
|
||||
| Active | 红色 | 伤害判定窗口 |
|
||||
| Recovery | 灰色 | 后摇 |
|
||||
| VulnerabilityWindow | 绿色覆盖 | 被弹反可反击窗口 |
|
||||
| DurationNormalized < 0.1 | 警告红 | 阶段过短，设计器警告 |
|
||||
|
||||
拖放 `BossSkillSO` / `SkillSequenceSO` 即可加载，`EditorGUIUtility.PingObject` 点击高亮资产——这是 Unity 原生编辑器工具的最佳实践写法。
|
||||
|
||||
---
|
||||
|
||||
### 4.4 AddressKeyValidator
|
||||
|
||||
```csharp
|
||||
public class AddressKeyValidatorBuildHook : IPreprocessBuildWithReport
|
||||
{
|
||||
public int callbackOrder => 0; // 在 SOValidationRunner(1) 之前执行
|
||||
|
||||
public void OnPreprocessBuild(BuildReport report)
|
||||
{
|
||||
// 反射枚举 AddressKeys 所有 const string 字段
|
||||
// 与 Addressable 分组实际地址集合做差集
|
||||
// 有孤儿 key → throw BuildFailedException → 构建中止
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**强制构建门槛**：孤儿 AddressKey 会导致运行时 `Addressables.LoadAssetAsync` 失败，这类错误在发布版本中极难排查。`IPreprocessBuildWithReport` 在构建流水线最早阶段拦截，与 CI/CD 自动化完全兼容。
|
||||
|
||||
---
|
||||
|
||||
### 4.5 SOValidationRunner + IValidatable（P3-9 修复后）
|
||||
|
||||
```csharp
|
||||
// 修复前：字符串启发式判断严重性
|
||||
bool isError = msg.Contains("必须") || msg.StartsWith("❌");
|
||||
|
||||
// 修复后：类型化严重性
|
||||
foreach (var result in validatable.Validate())
|
||||
{
|
||||
if (result.Severity == ValidationSeverity.Error)
|
||||
errors.Add($"❌ {result.Message} ({path})");
|
||||
else
|
||||
warnings.Add($"⚠️ {result.Message} ({path})");
|
||||
}
|
||||
```
|
||||
|
||||
严重性分级（Error/Warning）由 `ValidationResult` 结构体持有，消除了脆弱的字符串模式匹配。
|
||||
|
||||
---
|
||||
|
||||
### 4.6 HurtBoxEditor（P3-8 修复后）
|
||||
|
||||
```csharp
|
||||
// 修复前：反射读取 private 字段（字符串 fieldName，脆弱）
|
||||
// 修复后：typed lambda getter
|
||||
(System.Func<HurtBox, object> getter, string label, string absentNote)[] _fields = {
|
||||
(hb => hb.EditorOwner, "Owner (IDamageable)", "— 注入失败"),
|
||||
(hb => hb.EditorShieldable, "Shieldable", "— 无护盾"),
|
||||
(hb => hb.EditorParrySystem, "ParrySystem", "— 无弹反"),
|
||||
(hb => hb.EditorPoiseSource, "PoiseSource", "— 无霸体"),
|
||||
(hb => hb.EditorStatusEffectable,"StatusEffectable", "— 无状态效果"),
|
||||
};
|
||||
```
|
||||
|
||||
字段重命名后编译器立即报错，而非运行时 `null` 静默失败。
|
||||
|
||||
---
|
||||
|
||||
### 4.7 编辑器友好性待改进项
|
||||
|
||||
| 问题 | 建议 |
|
||||
|------|------|
|
||||
| `BossSkillSequenceWindow` 对 `_loadedSkill` 字段未作空字段检查 | 在 DrawSkillTimeline 入口添加 HelpBox 提示 |
|
||||
| `EventBusMonitor` 使用 `Queue` 而非固定 `EventRecord[]` | 换循环缓冲区，彻底消除 Editor GC |
|
||||
| `SOValidationRunner` 未提供 "一键修复" 按钮 | 对 Warning 级别问题提供可选自动修复 |
|
||||
| 无场景引用可视化工具 | 仿 Odin Inspector `[SceneObjectsOnly]` 属性或自定义 PropertyDrawer |
|
||||
|
||||
---
|
||||
|
||||
## 五、使用便利性（Developer Experience）
|
||||
|
||||
### 5.1 命名一致性
|
||||
|
||||
全项目命名规范高度一致：
|
||||
|
||||
| 约定 | 示例 |
|
||||
|------|------|
|
||||
| EventChannel SO：`_on` 前缀 | `_onPlayerDied`, `_onSaveIndicatorVisible` |
|
||||
| SO 类型后缀 | `InputReaderSO`, `ParryConfigSO`, `CharmEventChannelSO` |
|
||||
| 接口前缀 `I` | `IDamageable`, `ILOSRequester`, `ISaveable`, `IPlatformService` |
|
||||
| 管理器后缀 `Manager` | `EquipmentManager`, `SaveManager`, `StatusEffectManager` |
|
||||
| 枚举 `Type`/`Phase`/`Id` | `ParryPhase`, `GameStateId`, `StatusEffectType` |
|
||||
| 私有字段 `_camelCase` | `_currentSlot`, `_saveLock`, `_effectFactories` |
|
||||
|
||||
商业项目级别的命名一致性，新团队成员阅读代码时认知成本极低。
|
||||
|
||||
---
|
||||
|
||||
### 5.2 API 契约清晰度
|
||||
|
||||
**优秀范例：**
|
||||
|
||||
```csharp
|
||||
// TryEquipCharm：null = 成功，string = 错误原因（优于 bool + out string）
|
||||
public string TryEquipCharm(CharmSO charm) { ... }
|
||||
|
||||
// ConsumeJump/ConsumeAttack/ConsumeDash：读取即消耗，避免调用者手动清零
|
||||
public bool ConsumeJump() { ... }
|
||||
|
||||
// GetOrDefault：明确声明可能返回 null，不同于 Get 的快速失败语义
|
||||
public static T GetOrDefault<T>() { ... }
|
||||
|
||||
// ValidationResult.Error / ValidationResult.Warning：工厂方法减少直接 new
|
||||
public static ValidationResult Error(string msg) => new(ValidationSeverity.Error, msg);
|
||||
```
|
||||
|
||||
**需改进的 API：**
|
||||
|
||||
```csharp
|
||||
// HitBox.OnHitConfirmed 是 public field，非 event keyword
|
||||
// 外部可以用 = 覆盖所有订阅者
|
||||
public Action<DamageInfo> OnHitConfirmed; // ❌
|
||||
|
||||
// 应改为：
|
||||
public event Action<DamageInfo> OnHitConfirmed; // ✅
|
||||
|
||||
// PlayerController.GetCurrentPoiseLevel() 始终返回 PoiseLevel.None
|
||||
// 调用者无法区分"玩家本身无霸体设计"和"功能未实现"
|
||||
public PoiseLevel GetCurrentPoiseLevel() => PoiseLevel.None; // ❌ 误导性
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 5.3 订阅生命周期管理
|
||||
|
||||
```csharp
|
||||
// 推荐写法（CompositeDisposable 与 MonoBehaviour 生命周期绑定）
|
||||
private CompositeDisposable _subs = new();
|
||||
|
||||
private void OnEnable()
|
||||
{
|
||||
_onPlayerDied.Subscribe(OnPlayerDied).AddTo(_subs);
|
||||
_onRoomEntered.Subscribe(OnRoomEntered).AddTo(_subs);
|
||||
}
|
||||
|
||||
private void OnDisable() => _subs.Dispose();
|
||||
```
|
||||
|
||||
全项目统一了此模式，彻底解决了传统 `OnEnable += / OnDisable -=` 遗忘匹配的问题。这是区别于大多数中小型 Unity 项目的最大质量优势之一。
|
||||
|
||||
---
|
||||
|
||||
### 5.4 InputBuffer 设计
|
||||
|
||||
```csharp
|
||||
// 3 个独立帧缓冲，每帧递减，消耗即清零
|
||||
// 尺寸全部可在 Inspector 调节（不需要修改代码）
|
||||
[SerializeField] private float _jumpBufferDuration = 0.15f;
|
||||
[SerializeField] private float _attackBufferDuration = 0.12f;
|
||||
[SerializeField] private float _dashBufferDuration = 0.10f;
|
||||
```
|
||||
|
||||
`ConsumeJump()` / `ConsumeAttack()` / `ConsumeDash()` 的调用者不需要知道缓冲窗口时长，只需询问"现在能不能执行"。Celeste 的 Coyote Time 实现与此完全同构。
|
||||
|
||||
---
|
||||
|
||||
### 5.5 混用模式（-0.2 分）
|
||||
|
||||
```
|
||||
访问路径矛盾：
|
||||
ServiceLocator.Get<IObjectPoolService>() // GlobalObjectPool ✓
|
||||
GlobalObjectPool.Instance // GlobalObjectPool ✓（同一对象，两条路）
|
||||
SaveManager.Instance // SaveManager（不通过 ServiceLocator）
|
||||
VFXPool.Instance // VFXPool（不通过 ServiceLocator）
|
||||
ClashResolver → ServiceLocator.GetOrDefault<ClashResolver>() // ✓
|
||||
AudioManager → ???（已移除旧 .Instance，但新路径需确认）
|
||||
```
|
||||
|
||||
团队成员面对混用时难以判断"我该用哪个"，也会使单元测试的 Mock 替换复杂化。
|
||||
|
||||
---
|
||||
|
||||
### 5.6 使用便利性待改进项
|
||||
|
||||
| 问题 | 建议 |
|
||||
|------|------|
|
||||
| `HitBox.OnHitConfirmed` 为 public field | 改为 `public event Action<DamageInfo>` |
|
||||
| 混用 `.Instance` 单例 + ServiceLocator | 统一为 ServiceLocator，旧 `.Instance` 标记 `[Obsolete]` |
|
||||
| `DamageInfo` 非 `readonly struct` | 标记 `readonly`，修改操作改为 `With...()` 方法 |
|
||||
| SkillSlot 字符串魔法值 | 提取为 `static class SkillSlotNames` 常量 |
|
||||
| `AnalyticsManager` 无 namespace | 添加 `namespace BaseGames.Support.Analytics` |
|
||||
|
||||
---
|
||||
|
||||
## 六、商业基准对标
|
||||
|
||||
| 维度 | zeling_v2 | Hollow Knight（估算） | Celeste（开源代码） | Hades（GDC 演讲） |
|
||||
|------|-----------|----------------------|--------------------|--------------------|
|
||||
| 程序集隔离 | ✅ 25 asmdef | ✅ 多 asmdef | ❌ 单项目 | ✅ 分层 |
|
||||
| 事件系统 | ✅ SO Channel + IDisposable | ✅ 自定义事件总线 | ✅ Celeste 事件系统 | ✅ 消息总线 |
|
||||
| 零分配热路径 | ✅ struct DamageInfo | ✅ struct 伤害值 | ✅ 简单值类型 | ✅ 严格零分配 |
|
||||
| 时间切片 AI | ✅ BatchLOSSystem | ✅ 视野感知分帧 | N/A | ✅ 模式分帧 |
|
||||
| 数据驱动护符 | ✅ CharmSO + ICharmEffect | ✅ Charm 系统 | N/A | ✅ Boon SO |
|
||||
| 存档版本迁移 | ✅ goto 链 | ✅ 版本号检查 | ✅ | ✅ |
|
||||
| 编辑器工具链 | ✅ 4 专用工具 | 未知 | ✅ Lönn 编辑器 | ✅ 内部工具 |
|
||||
| 弹反系统完备性 | ✅ 5 相位状态机 | ✅ 经典弹反 | N/A | ✅ 多弹反类型 |
|
||||
| 模式一致性 | ⚠️ 混用单例 | ✅ 统一单例 | ✅ 统一单例 | ✅ 统一 SL |
|
||||
|
||||
---
|
||||
|
||||
## 七、综合建议
|
||||
|
||||
### 高优先级（影响可维护性）
|
||||
|
||||
1. **统一服务访问模式**：`SaveManager`、`VFXPool` 注册到 `ServiceLocator`，`.Instance` 添加 `[Obsolete]`。
|
||||
2. **`HitBox.OnHitConfirmed` 改为 `event`**：消除外部覆盖风险，影响范围小。
|
||||
3. **`AnalyticsManager` 添加 namespace**：`BaseGames.Support.Analytics`，5 分钟可完成。
|
||||
4. **`BatchLOSSystem._requesters` 改 HashSet**：场景大规模加载时性能优化。
|
||||
|
||||
### 中优先级（影响代码质量）
|
||||
|
||||
5. **`PlayerController.GetCurrentPoiseLevel()` 实现或标记未完成**。
|
||||
6. **`DamageInfo` 标记为 `readonly struct`**，Builder 内使用局部变量。
|
||||
7. **`IGameState.ValidNextStates` 改为 `IReadOnlySet<GameStateId>`**。
|
||||
8. **`EquipmentManager.UsedNotches` 缓存计算结果**，避免每次调用 LINQ `Sum()`。
|
||||
|
||||
### 低优先级（技术债偿还）
|
||||
|
||||
9. **SaveMigrator 版本字符串改为常量** `const string V1_0 = "1.0"`，消除 magic string。
|
||||
10. **SkillSlot 字符串统一到 `SkillSlotNames` 常量类**。
|
||||
11. **EventBusMonitor 改用固定 `EventRecord[]` 环形缓冲区**（消除 Editor GC）。
|
||||
|
||||
---
|
||||
|
||||
## 附录：文件覆盖说明
|
||||
|
||||
本次评审直接阅读的源文件（按模块）：
|
||||
|
||||
| 模块 | 已审文件 |
|
||||
|------|---------|
|
||||
| Core/Events | `BaseEventChannelSO.cs`, `EventSubscription.cs`, `EventBusMonitor.cs` |
|
||||
| Core/Save | `SaveManager.cs`, `SaveMigrator.cs`, `ISaveable.cs`, `WorldStateRegistry.cs` |
|
||||
| Core/Pool | `GlobalObjectPool.cs`, `PooledObject.cs` |
|
||||
| Core/Assets | `AssetLoader.cs`, `AssetReleaseTracker.cs` |
|
||||
| Core | `ServiceLocator.cs`, `GameStateMachine.cs` |
|
||||
| Input | `InputReaderSO.cs`, `InputBuffer.cs` |
|
||||
| Player | `PlayerController.cs`, `PlayerStateBase.cs`, `PlayerMovement.cs` |
|
||||
| Combat | `HurtBox.cs`, `HitBox.cs`, `DamageInfo.cs`, `ClashResolver.cs` |
|
||||
| Combat/StatusEffects | `StatusEffectManager.cs` |
|
||||
| Enemies | `EnemyBase.cs`, `BossBase.cs`, `BatchLOSSystem.cs` |
|
||||
| Equipment | `EquipmentManager.cs` |
|
||||
| Skills | `SkillModifierRegistry.cs` |
|
||||
| Parry | `ParrySystem.cs` |
|
||||
| VFX | `VFXPool.cs` |
|
||||
| Support | `AnalyticsManager.cs`, `SteamPlatformService.cs` |
|
||||
| Editor | `EventBusMonitorWindow.cs`, `BossSkillSequenceWindow.cs`, `AddressKeyValidator.cs`, `HurtBoxEditor.cs`, `SOValidationRunner.cs` |
|
||||
|
||||
> 受覆盖范围限制，`Dialogue`、`Quest`、`Cutscene`、`Tutorial`、`Localization` 等子系统未纳入本次深度审查。
|
||||
|
||||
---
|
||||
|
||||
*生成于 2026-01 | 评审人：GitHub Copilot (Claude Sonnet 4.6)*
|
||||
Reference in New Issue
Block a user