284 lines
8.7 KiB
Markdown
284 lines
8.7 KiB
Markdown
# FlowScope P2 前整改复审意见
|
||
|
||
日期：2026-05-21
|
||
审查对象：`558bc91 完成 P2 前架构整改`
|
||
审查范围：Container 生命周期、GameFlow/Feature 生命周期、MainMenuP0 Bootstrap、UIPreloadService、ResourceService 取消语义、相关文档与测试。
|
||
审查结论：本轮整改方向正确，已收口上一轮最关键的 Container/Feature 重入问题；但仍有两个 P1 级风险需要在进入 P2 前修正。
|
||
|
||
---
|
||
|
||
## 本轮已确认收口
|
||
|
||
### 1. Container 生命周期语义已明显改善
|
||
|
||
当前实现新增并区分：
|
||
|
||
- `RegisterSingletonFactory`
|
||
- `RegisterScoped`
|
||
- `RegisterTransient`
|
||
|
||
`RegisterFactory` 保留为旧式 lazy singleton 兼容入口，文档中已提示不要用于 Feature 或临时业务对象。
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Runtime/Container/Container.cs`
|
||
- `My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs`
|
||
- `My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs`
|
||
|
||
新增测试覆盖：
|
||
|
||
- singleton 由注册 scope 持有。
|
||
- transient 每次 resolve 创建新实例，并由请求 scope 释放。
|
||
- scoped 在同一请求 scope 内复用，不同 scope 分离。
|
||
|
||
评价：上一轮指出的“`RegisterFactory` 语义误导 + Feature 复用已 Dispose 实例”问题已经被大幅降低。
|
||
|
||
---
|
||
|
||
### 2. Feature 重入问题已补测试
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs`
|
||
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
|
||
|
||
已新增测试：
|
||
|
||
- `SwitchToAsync_WhenReturningToFeatureType_CreatesNewFeatureInstance`
|
||
|
||
样例中 `MainMenuFeature` 已改为：
|
||
|
||
```csharp
|
||
_root.RegisterTransient(_ => new MainMenuFeature(screenNavigator, _playerData));
|
||
```
|
||
|
||
评价：`A -> B -> A` 场景已经有明确保护，符合 P2 前最低要求。
|
||
|
||
---
|
||
|
||
### 3. Bootstrap 启动入口可观察性已改善
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
|
||
- `My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs`
|
||
|
||
已从 `async void Awake()` 直接 await 改为：
|
||
|
||
- `Awake()` 创建 lifetime token。
|
||
- `StartupTask = RunStartupWithLoggingAsync(...)`。
|
||
- PlayMode 测试覆盖 `Awake_StartsThroughObservableStartupTask`。
|
||
|
||
评价：启动异常现在可以通过 `StartupTask` 被观察，样例不再鼓励直接在 `Awake` 中 await。
|
||
|
||
---
|
||
|
||
### 4. ResourceService 共享加载取消边界已有补测
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs`
|
||
|
||
新增测试覆盖：
|
||
|
||
- 一个共享等待者取消，另一个等待者仍能拿到 handle。
|
||
- 所有共享等待者取消后，底层完成时释放 backend asset，不进入 completed cache。
|
||
|
||
评价：资源层的“调用者取消不取消共享底层加载”语义更清楚了。
|
||
|
||
---
|
||
|
||
## 仍需修正的问题
|
||
|
||
### P1. `GameBootstrap.ShutdownAsync` 失败后会永久跳过后续清理
|
||
|
||
风险等级：高
|
||
建议：进入 P2 前修正。
|
||
|
||
证据：
|
||
|
||
文件：`My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
|
||
|
||
当前流程：
|
||
|
||
```csharp
|
||
if (_shutdownStarted)
|
||
{
|
||
return;
|
||
}
|
||
|
||
_shutdownStarted = true;
|
||
|
||
await _saveService.SaveAsync(...);
|
||
await _gameFlow.ShutdownAsync(...);
|
||
_preloadService?.ReleaseAll();
|
||
_shutdown = true;
|
||
```
|
||
|
||
问题：
|
||
|
||
- `_shutdownStarted = true` 在保存、GameFlow 关闭和预加载释放之前设置。
|
||
- 如果 `SaveAsync` 抛异常，`GameFlow.ShutdownAsync` 不会执行。
|
||
- 如果 `GameFlow.ShutdownAsync` 抛异常，`_preloadService.ReleaseAll()` 不会执行。
|
||
- 因为 `_shutdownStarted` 已经是 true，之后再次调用 `ShutdownAsync` 会直接 return。
|
||
- 结果是 `_shutdown` 不会置 true，预加载资源和 runtime 对象可能残留。
|
||
|
||
建议修法：
|
||
|
||
1. 将“正在关闭”和“已完成关闭”拆成不同状态。
|
||
2. 关闭流程应使用 best-effort / finally：
|
||
- 尽量保存。
|
||
- 尽量关闭 GameFlow。
|
||
- 无论前面是否失败，都释放预加载资源。
|
||
- 最后根据结果记录异常或聚合异常。
|
||
3. 如果第一次关闭失败，应允许再次尝试未完成的清理，或者至少确保清理已经执行过。
|
||
|
||
建议补测：
|
||
|
||
- Save 失败时，仍会执行 GameFlow shutdown 和 preload release。
|
||
- GameFlow shutdown 失败时，仍会执行 preload release。
|
||
- Shutdown 失败后再次调用不会直接跳过必要清理。
|
||
|
||
---
|
||
|
||
### P1. `UIPreloadService` 并发去重会把首个调用者取消传播给所有等待者
|
||
|
||
风险等级：高
|
||
建议：进入 P2 前修正。
|
||
|
||
证据：
|
||
|
||
文件：`My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
|
||
|
||
当前流程：
|
||
|
||
```csharp
|
||
if (_inFlightLoads.TryGetValue(panelType, out var inFlightLoad))
|
||
{
|
||
await inFlightLoad;
|
||
return;
|
||
}
|
||
|
||
var loadTask = LoadAndStoreAsync(panelType, path, cancellationToken);
|
||
_inFlightLoads.Add(panelType, loadTask);
|
||
await loadTask;
|
||
```
|
||
|
||
问题：
|
||
|
||
- 首个调用者的 `cancellationToken` 被用于底层 `LoadAndStoreAsync`。
|
||
- 后续调用者只 await 同一个 `inFlightLoad`。
|
||
- 如果第一个调用者取消，后续未取消的调用者也会失败。
|
||
- 这与 `ResourceService` 已经建立的共享加载语义不一致：调用者取消应只取消自己的等待，不应取消共享底层加载。
|
||
|
||
建议修法：
|
||
|
||
1. `_inFlightLoads` 不应直接绑定首个调用者的 token。
|
||
2. 底层 preload load 可使用独立生命周期 token 或不可取消共享任务。
|
||
3. 每个调用者自己的 token 只控制自己的等待。
|
||
4. 如果所有等待者都取消，再决定是否取消或释放底层任务，语义要和 ResourceService 对齐。
|
||
|
||
建议补测：
|
||
|
||
- 第一个 `PreloadAsync<TPanel>(cts.Token)` 取消，第二个 `PreloadAsync<TPanel>(CancellationToken.None)` 仍成功。
|
||
- 第二个等待者取消，不影响第一个等待者成功。
|
||
- 所有等待者取消后，最终 handle 不被缓存或被正确释放。
|
||
|
||
---
|
||
|
||
## 仍未完全闭环但可后置的风险
|
||
|
||
### Addressables 真实 load/release 验证仍然较薄
|
||
|
||
当前 `AddressablesResourceBackendTests` 仍主要是 `Name_ReturnsAddressables`。
|
||
这不是本轮整改新引入的问题，但它仍然是 P2 包分发前的验收缺口。
|
||
|
||
建议：
|
||
|
||
- 如果短期不方便制作 Addressables 测试资源，至少补一条人工验收记录。
|
||
- 若准备进入 P2 包生态，建议补最小 Addressables PlayMode 集成测试：
|
||
- load 成功。
|
||
- release 被调用。
|
||
- load 失败路径可诊断。
|
||
- caller cancellation 语义与 ResourceService 不冲突。
|
||
|
||
---
|
||
|
||
## 文档状态
|
||
|
||
本轮已更新：
|
||
|
||
- `docs/guides/flowscope-runtime-usage.md`
|
||
- `docs/requirements/p1-production-hardening.md`
|
||
- `docs/reviews/flowscope-framework-review.md`
|
||
- `docs/reviews/p1-completion-report.md`
|
||
- `docs/reviews/p1-architecture-audit-before-p2.md`
|
||
|
||
评价：
|
||
|
||
- P1 completion report 中旧的 Addressables 编译阻塞状态已经同步。
|
||
- runtime usage guide 已明确 `RegisterFactory` 是兼容 lazy singleton，不推荐用于 Feature。
|
||
- 但上一份 `p1-architecture-audit-before-p2.md` 仍保留原始问题主体，同时在顶部记录“第一轮 P2 前最小整改”。这可以接受，因为它是历史审核文档；后续执行应以本文的复审问题为准。
|
||
|
||
---
|
||
|
||
## 最新轻量验证
|
||
|
||
本次复审后执行：
|
||
|
||
```powershell
|
||
dotnet build "My project\FlowScope.Tests.EditMode.csproj" --no-restore
|
||
dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore
|
||
```
|
||
|
||
结果：
|
||
|
||
- EditMode generated csproj build：0 error。
|
||
- PlayMode generated csproj build：0 error。
|
||
- 仍有 Unity 生成 csproj 的引用版本冲突 warning。
|
||
|
||
说明：
|
||
|
||
- 以上验证只能证明生成的 C# 项目可编译。
|
||
- 本文未重新执行 Unity Test Runner。
|
||
- 本文未做 MainMenuP0 人工场景验收。
|
||
|
||
---
|
||
|
||
## P2 Gate 判断
|
||
|
||
当前不建议立刻进入 P2。
|
||
|
||
原因：
|
||
|
||
1. `ShutdownAsync` 的失败清理问题会影响样例作为官方 Bootstrap 模板的可信度。
|
||
2. `UIPreloadService` 并发取消语义与 ResourceService 不一致，属于框架使用者容易踩到的生命周期问题。
|
||
|
||
建议最小进入 P2 条件：
|
||
|
||
1. 修复 `GameBootstrap.ShutdownAsync` best-effort 清理。
|
||
2. 修复 `UIPreloadService` per-caller cancellation 与共享 preload 语义。
|
||
3. 补对应 EditMode / PlayMode 测试。
|
||
4. 保持当前 generated csproj build 0 error。
|
||
5. 如果 P2 第一项是 package/editor tooling，至少补一次 Unity Test Runner 或明确记录无法执行原因。
|
||
|
||
---
|
||
|
||
## 后续执行建议
|
||
|
||
优先级顺序：
|
||
|
||
1. 先修 `ShutdownAsync` 的失败清理和重入状态。
|
||
2. 再修 `UIPreloadService` 的 in-flight cancellation 语义。
|
||
3. 补测试。
|
||
4. 再讨论是否进入 P2。
|
||
|
||
不要在这轮顺手扩展：
|
||
|
||
- Luban。
|
||
- YooAsset / AssetBundle。
|
||
- 云存档。
|
||
- AudioMixer / 3D 音频。
|
||
- 完整 UI 路由。
|
||
- Package 分发和编辑器工具。
|