220 lines
6.8 KiB
Markdown
220 lines
6.8 KiB
Markdown
# FlowScope P2 前第三轮复审意见
|
||
|
||
日期：2026-05-21
|
||
审查对象：`e211a71 修复 UI 预加载同步完成缓存问题` 及其前置整改提交
|
||
审查范围：`GameBootstrap.ShutdownAsync`、`UIPreloadService` 并发/取消/释放语义、相关 PlayMode 测试、generated csproj 编译状态。
|
||
审查结论：上一轮两个 P1 gate 已基本收口；当前进入 P2 前只剩一个 UI preload 生命周期边界建议先修。
|
||
|
||
---
|
||
|
||
## 本轮确认已收口
|
||
|
||
### 1. Shutdown best-effort 清理已落地
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
|
||
- `My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs`
|
||
|
||
当前 `ShutdownAsync` 已经拆分：
|
||
|
||
- `_shutdownInProgress`
|
||
- `_shutdown`
|
||
- `exceptions`
|
||
|
||
并且按 best-effort 顺序执行：
|
||
|
||
1. 尝试保存。
|
||
2. 尝试关闭 `GameFlow`。
|
||
3. 无论前面是否失败，仍尝试 `ReleaseAll()`。
|
||
4. 最后再重新抛出单个异常或聚合异常。
|
||
|
||
对应新增测试：
|
||
|
||
- `Shutdown_WhenSaveFails_StillShutsDownFlowAndReleasesPreloads`
|
||
- `Shutdown_WhenGameFlowShutdownFails_StillReleasesPreloads`
|
||
- `Shutdown_AfterFailure_DoesNotSkipCleanupOnSecondCall`
|
||
|
||
评价：上一轮指出的“第一次 shutdown 失败后永久跳过后续清理”问题已基本修正。
|
||
|
||
---
|
||
|
||
### 2. UIPreloadService 首个调用者取消传播问题已修正
|
||
|
||
相关文件：
|
||
|
||
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
|
||
- `My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs`
|
||
|
||
当前实现变化：
|
||
|
||
- `_inFlightLoads` 从 `Task` 改为 `InFlightPreload`。
|
||
- 底层 `_resources.LoadAsync<GameObject>` 使用 `CancellationToken.None`。
|
||
- 调用者自己的 token 只影响自己的等待。
|
||
- `WaiterCount` 用来判断最终 handle 是否应缓存。
|
||
|
||
对应新增测试：
|
||
|
||
- `PreloadAsync_WhenFirstWaiterCancels_SecondWaiterStillSucceeds`
|
||
- `PreloadAsync_WhenSecondWaiterCancels_FirstWaiterStillSucceeds`
|
||
- `PreloadAsync_WhenAllWaitersCancel_ReleasesLoadedHandleWithoutCaching`
|
||
- `PreloadAsync_WhenSamePanelLoadsConcurrently_UsesOneResourceLoad`
|
||
|
||
评价：上一轮指出的“首个 caller 取消会影响其他 waiter”问题已修正。
|
||
|
||
---
|
||
|
||
### 3. 同步完成缓存问题已修正
|
||
|
||
最新提交 `e211a71 修复 UI 预加载同步完成缓存问题` 对 `UIPreloadService` 做了补充修正。
|
||
|
||
评价：这个修正方向是正确的，说明当前实现已经考虑到资源加载可能同步完成，不再只覆盖 gated async path。
|
||
|
||
---
|
||
|
||
## 仍建议 P2 前修正的问题
|
||
|
||
### P1. `UIPreloadService.ReleaseAll/Dispose` 未处理 in-flight preload
|
||
|
||
风险等级：高
|
||
建议：进入 P2 前修正。
|
||
|
||
证据文件：
|
||
|
||
- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs`
|
||
|
||
当前 `ReleaseAll()` 只处理 `_handles`：
|
||
|
||
```csharp
|
||
public void ReleaseAll()
|
||
{
|
||
List<IResourceHandle<GameObject>> handles;
|
||
lock (_gate)
|
||
{
|
||
handles = new List<IResourceHandle<GameObject>>(_handles.Values);
|
||
_handles.Clear();
|
||
}
|
||
|
||
foreach (var handle in handles)
|
||
{
|
||
handle.Dispose();
|
||
}
|
||
}
|
||
```
|
||
|
||
但 `LoadAndStoreAsync()` 完成后仍可能写回 `_handles`：
|
||
|
||
```csharp
|
||
lock (_gate)
|
||
{
|
||
_inFlightLoads.Remove(panelType);
|
||
if (inFlightLoad.WaiterCount > 0)
|
||
{
|
||
_handles[panelType] = handle;
|
||
shouldCache = true;
|
||
}
|
||
}
|
||
```
|
||
|
||
问题场景：
|
||
|
||
1. `PreloadAsync<TPanel>()` 已启动，底层资源仍在加载。
|
||
2. 场景退出或 bootstrap shutdown 调用 `ReleaseAll()` / `Dispose()`。
|
||
3. `ReleaseAll()` 清空已缓存 handle，但不会标记服务已释放，也不会处理 `_inFlightLoads`。
|
||
4. 底层 load 随后完成。
|
||
5. 因为 `WaiterCount > 0`，`LoadAndStoreAsync()` 仍会把 handle 写回 `_handles`。
|
||
|
||
结果：
|
||
|
||
- 服务已经被释放后，异步完成又重新缓存资源。
|
||
- shutdown 之后可能残留 handle。
|
||
- P2 若加入 package/sample/editor tooling，这种生命周期反转会变成用户很难定位的泄漏或幽灵状态。
|
||
|
||
建议修法：
|
||
|
||
1. 增加 `_disposed` 或 `_releaseGeneration` 状态。
|
||
2. `ReleaseAll()` / `Dispose()` 时：
|
||
- 清空 `_handles`。
|
||
- 标记当前 service 不再接受缓存写回。
|
||
- 对 `_inFlightLoads` 做明确处理：要么取消 owner token，要么让完成后的 handle 直接 dispose。
|
||
3. `PreloadAsync()` 在 service disposed 后应明确抛 `ObjectDisposedException` 或直接拒绝新请求，避免释放后继续使用。
|
||
4. `LoadAndStoreAsync()` 完成时必须检查 release/dispose 状态；如果已释放，dispose handle，不缓存。
|
||
|
||
建议补测：
|
||
|
||
- `ReleaseAll_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
|
||
- `Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`
|
||
- `PreloadAsync_AfterDispose_ThrowsObjectDisposedException`
|
||
|
||
---
|
||
|
||
## 中风险说明
|
||
|
||
### `IsShutdown` 语义需要避免误读
|
||
|
||
风险等级：中
|
||
建议：可在 P2 前顺手明确文档或命名。
|
||
|
||
当前 `GameBootstrap.ShutdownAsync` 即使 `SaveAsync` 或 `GameFlow.ShutdownAsync` 抛错，也会在完成 best-effort cleanup 后设置：
|
||
|
||
```csharp
|
||
_shutdown = true;
|
||
```
|
||
|
||
然后再向调用方抛出异常。
|
||
|
||
这个行为本身可以接受，因为它表达的是“runtime cleanup 已完成”，不是“所有 shutdown 子步骤都成功”。
|
||
但 `IsShutdown` 容易被读成“关闭成功”。建议在样例文档或属性注释里明确：
|
||
|
||
- `IsShutdown` 表示 MainMenuP0 runtime cleanup 已结束。
|
||
- 不表示保存一定成功。
|
||
- 调用者仍应以 `ShutdownAsync` 是否抛异常判断保存/关闭子步骤是否失败。
|
||
|
||
---
|
||
|
||
## 仍可后置的风险
|
||
|
||
### Addressables 真实 load/release 验证仍薄
|
||
|
||
这不是本轮整改新引入的问题，但仍是 P2 包分发前的验收缺口。
|
||
|
||
当前已有 generated csproj 编译覆盖，但 Addressables 适配层真实资源加载、释放、失败诊断仍缺少较强测试。
|
||
如果 P2 先做 package 结构或文档，可以后置；如果 P2 要直接推广资源后端能力，建议补最小 PlayMode 集成或人工验收记录。
|
||
|
||
---
|
||
|
||
## 本轮验证
|
||
|
||
执行：
|
||
|
||
```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 generated csproj 常见引用冲突 warning。
|
||
|
||
说明：
|
||
|
||
- 本轮未执行 Unity Test Runner。
|
||
- 本轮未做 MainMenuP0 人工场景验收。
|
||
|
||
---
|
||
|
||
## P2 Gate 判断
|
||
|
||
当前建议：暂缓进入 P2，先修一个最小问题。
|
||
|
||
P2 前最小清单：
|
||
|
||
1. 修复 `UIPreloadService.ReleaseAll/Dispose` 与 in-flight preload 的生命周期边界。
|
||
2. 补 in-flight release/dispose 对应 PlayMode 测试。
|
||
3. 保持 EditMode / PlayMode generated csproj build 0 error。
|
||
|
||
完成后，P2 gate 可以认为基本通过。届时进入 P2 前只需再决定 Addressables 真实集成验收是 P2 前置，还是 P2 内第一项。
|
||
|