From 7cde2cfaec1d82a57392102858d40ce7a15b51fb Mon Sep 17 00:00:00 2001 From: "JSD\\13999" <1399945104@qq.com> Date: Thu, 21 May 2026 17:00:16 +0800 Subject: [PATCH] =?UTF-8?q?=E6=94=B6=E5=8F=A3=20P2=20=E5=89=8D=E9=A2=84?= =?UTF-8?q?=E5=8A=A0=E8=BD=BD=E8=BE=B9=E7=95=8C=E5=B9=B6=E8=A1=A5=E9=9C=80?= =?UTF-8?q?=E6=B1=82=E6=96=87=E6=A1=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../FlowScope/Runtime/UI/UIPreloadService.cs | 40 +++- .../PlayMode/UI/UIPreloadServiceTests.cs | 139 ++++++++++++ .../p2-package-editor-requirements.md | 163 ++++++++++++++ docs/reviews/p2-precheck-fifth-review.md | 184 ++++++++++++++++ docs/reviews/p2-precheck-fourth-review.md | 207 ++++++++++++++++++ 5 files changed, 723 insertions(+), 10 deletions(-) create mode 100644 docs/requirements/p2-package-editor-requirements.md create mode 100644 docs/reviews/p2-precheck-fifth-review.md create mode 100644 docs/reviews/p2-precheck-fourth-review.md diff --git a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs index 6776850..4a59c70 100644 --- a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs +++ b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs @@ -38,13 +38,15 @@ namespace FlowScope.UI return; } - if (!_inFlightLoads.TryGetValue(panelType, out inFlightLoad)) + if (!_inFlightLoads.TryGetValue(panelType, out inFlightLoad) || + inFlightLoad.Invalidated || + inFlightLoad.ReleaseGeneration != _releaseGeneration) { var attribute = UIManager.GetPanelAttribute(panelType); path = UIManager.ResolvePath(panelType, attribute); inFlightLoad = new InFlightPreload(); inFlightLoad.ReleaseGeneration = _releaseGeneration; - _inFlightLoads.Add(panelType, inFlightLoad); + _inFlightLoads[panelType] = inFlightLoad; startLoad = true; } @@ -96,7 +98,7 @@ namespace FlowScope.UI public void Release() { var panelType = typeof(TPanel); - IResourceHandle handle; + IResourceHandle handle = null; lock (_gate) { if (_disposed) @@ -104,15 +106,14 @@ namespace FlowScope.UI return; } - if (!_handles.TryGetValue(panelType, out handle)) + InvalidateInFlightLoad(panelType); + if (_handles.TryGetValue(panelType, out handle)) { - return; + _handles.Remove(panelType); } - - _handles.Remove(panelType); } - handle.Dispose(); + handle?.Dispose(); } public void ReleaseAll() @@ -164,8 +165,9 @@ namespace FlowScope.UI var shouldCache = false; lock (_gate) { - _inFlightLoads.Remove(panelType); + ForgetInFlightLoad(panelType, inFlightLoad); if (!_disposed && + !inFlightLoad.Invalidated && inFlightLoad.ReleaseGeneration == _releaseGeneration && inFlightLoad.WaiterCount > 0) { @@ -183,13 +185,30 @@ namespace FlowScope.UI { lock (_gate) { - _inFlightLoads.Remove(panelType); + ForgetInFlightLoad(panelType, inFlightLoad); } throw; } } + private void ForgetInFlightLoad(Type panelType, InFlightPreload inFlightLoad) + { + if (_inFlightLoads.TryGetValue(panelType, out var current) && + ReferenceEquals(current, inFlightLoad)) + { + _inFlightLoads.Remove(panelType); + } + } + + private void InvalidateInFlightLoad(Type panelType) + { + if (_inFlightLoads.TryGetValue(panelType, out var inFlightLoad)) + { + inFlightLoad.Invalidated = true; + } + } + private void ForgetWaitingCaller(Type panelType, InFlightPreload inFlightLoad) { lock (_gate) @@ -249,6 +268,7 @@ namespace FlowScope.UI public Task Task { get; set; } public int WaiterCount { get; set; } public int ReleaseGeneration { get; set; } + public bool Invalidated { get; set; } } } } diff --git a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs index 21c9327..f151822 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs @@ -173,6 +173,68 @@ namespace FlowScope.Tests.PlayMode.UI Assert.That(preload.IsPreloaded(), Is.False); } + [UnityTest] + public IEnumerator PreloadAsync_AfterReleaseAllWhileOldLoadInFlight_StartsNewLoadAndCachesNewHandle() + { + var oldPrefab = CreatePrefab("OldExplicitPathPrefab"); + var newPrefab = CreatePrefab("NewExplicitPathPrefab"); + var resources = new SequencedGatedUITestResources("Custom/Explicit", oldPrefab, newPrefab); + var preload = new UIPreloadService(resources); + + var oldTask = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + + preload.ReleaseAll(); + + var newTask = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(2)); + + resources.Complete(0); + yield return UITestAsync.Await(oldTask); + + Assert.That(resources.HandleAt(0).IsDisposed, Is.True); + Assert.That(preload.IsPreloaded(), Is.False); + + resources.Complete(1); + yield return UITestAsync.Await(newTask); + + Assert.That(resources.HandleAt(1).IsDisposed, Is.False); + Assert.That(preload.TryGetPreloadedHandle(out var handle), Is.True); + Assert.That(handle.Asset, Is.SameAs(newPrefab)); + } + + [UnityTest] + public IEnumerator OldInFlightCompletion_AfterReleaseAll_DoesNotRemoveNewInFlight() + { + var oldPrefab = CreatePrefab("OldExplicitPathPrefab"); + var newPrefab = CreatePrefab("NewExplicitPathPrefab"); + var resources = new SequencedGatedUITestResources("Custom/Explicit", oldPrefab, newPrefab); + var preload = new UIPreloadService(resources); + + var oldTask = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + + preload.ReleaseAll(); + + var newTask = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(2)); + + resources.Complete(0); + yield return UITestAsync.Await(oldTask); + + Assert.That(preload.IsPreloaded(), Is.False); + + var thirdTask = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(2)); + + resources.Complete(1); + yield return UITestAsync.Await(newTask); + yield return UITestAsync.Await(thirdTask); + + Assert.That(resources.HandleAt(1).IsDisposed, Is.False); + Assert.That(preload.IsPreloaded(), Is.True); + } + [UnityTest] public IEnumerator Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle() { @@ -244,6 +306,24 @@ namespace FlowScope.Tests.PlayMode.UI Assert.That(preload.IsPreloaded(), Is.False); } + [UnityTest] + public IEnumerator Release_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle() + { + var prefab = CreatePrefab("ExplicitPathPrefab"); + var resources = new GatedUITestResources("Custom/Explicit", prefab); + var preload = new UIPreloadService(resources); + + var task = preload.PreloadAsync(CancellationToken.None); + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + + preload.Release(); + resources.Complete(); + yield return UITestAsync.Await(task); + + Assert.That(resources.Handle.IsDisposed, Is.True); + Assert.That(preload.IsPreloaded(), Is.False); + } + private Canvas CreateCanvas(string name) { var gameObject = new GameObject(name); @@ -313,5 +393,64 @@ namespace FlowScope.Tests.PlayMode.UI return (FlowScope.Resources.IResourceHandle)(object)handle; } } + + private sealed class SequencedGatedUITestResources : FlowScope.Resources.IResourceService + { + private readonly string _key; + private readonly List _assets = new(); + private readonly List> _handles = new(); + private readonly List>> _gates = new(); + + public SequencedGatedUITestResources(string key, params GameObject[] assets) + { + _key = key; + foreach (var asset in assets) + { + _assets.Add(asset); + _handles.Add(new UITestResourceHandle(key, asset)); + _gates.Add(new TaskCompletionSource>( + TaskCreationOptions.RunContinuationsAsynchronously)); + } + } + + public int LoadCalls { get; private set; } + + public UITestResourceHandle HandleAt(int index) + { + return _handles[index]; + } + + public Task> LoadAsync( + string key, + CancellationToken cancellationToken) + where T : class + { + cancellationToken.ThrowIfCancellationRequested(); + Assert.That(key, Is.EqualTo(_key)); + + var loadIndex = LoadCalls; + LoadCalls++; + Assert.That(loadIndex, Is.LessThan(_gates.Count), "Unexpected extra resource load."); + return AwaitHandle(loadIndex); + } + + public FlowScope.Resources.IResourceGroup CreateGroup() + { + throw new System.NotSupportedException(); + } + + public void Complete(int index) + { + _gates[index].TrySetResult(_handles[index]); + } + + private async Task> AwaitHandle(int index) + where T : class + { + var handle = await _gates[index].Task; + Assert.That(handle.Asset, Is.SameAs(_assets[index])); + return (FlowScope.Resources.IResourceHandle)(object)handle; + } + } } } diff --git a/docs/requirements/p2-package-editor-requirements.md b/docs/requirements/p2-package-editor-requirements.md new file mode 100644 index 0000000..542d3df --- /dev/null +++ b/docs/requirements/p2-package-editor-requirements.md @@ -0,0 +1,163 @@ +# FlowScope Game Core P2 需求文档 + +## 文档目的 + +本文用于开启 P2 新对话时作为需求入口。P2 不再继续修补 P0/P1 内核语义，而是在当前已稳定的 Runtime 基础上，推进包分发、编辑器体验、真实集成验收和跨项目复用。 + +## 当前基线 + +- P0 已提供 Container、Feature、GameFlow、ResourceService、ConfigProvider、SaveService、AudioService、UIManager 等轻量核心能力。 +- P1 已完成生产化第一步：资源后端抽象、Addressables 独立适配、配置 source/parser、存档 migration、UI 导航与预加载、MainMenuP0 样例升级。 +- P2 前整改已收口 Container 生命周期、Feature scope、GameFlow shutdown、ResourceService 共享加载、UIPreloadService 取消/释放/重入边界。 +- 进入 P2 前，EditMode 与 PlayMode Unity Test Runner 已由人工确认全绿。 + +## P2 总目标 + +把 FlowScope 从“项目内 Runtime 骨架”推进到“可跨项目复用的 Unity Package 雏形”，让使用者能清楚安装、接入、验证、诊断，并避免误用 P0/P1 核心 API。 + +## 核心原则 + +- 不重写 P0/P1 核心生命周期。 +- 不引入大型 DI、全局 EventBus、完整生态或多后端大一统。 +- 先建立分发和验收边界，再扩展能力。 +- Runtime、Addressables、Editor、Samples、Tests 之间保持 asmdef 边界清晰。 +- 任何新增 API 必须有使用场景、测试和文档，不为了“框架完整感”提前扩张。 + +## 本轮范围 + +### 1. Package 分发结构 + +目标：让 FlowScope 可以被整理成标准 Unity Package 形态，并保留当前 Assets 内开发体验的迁移路径。 + +需求： + +- 定义包名、版本号、displayName、Unity 版本和依赖。 +- 明确 Runtime、Addressables adapter、Editor、Samples、Tests 的目录结构。 +- 明确 asmdef 依赖方向： + - `FlowScope.Runtime` 不依赖 Addressables、Editor、Samples。 + - `FlowScope.Addressables` 依赖 `FlowScope.Runtime` 与 Unity Addressables。 + - `FlowScope.Editor` 只依赖需要检查的 runtime/editor 程序集。 + - Samples 依赖 Runtime 与需要演示的适配包。 +- 给出从 `Assets/FlowScope` 迁移到 package 结构的阶段方案，避免一次性大搬迁破坏现有测试。 + +验收： + +- package 元数据完整。 +- Unity 能识别 package。 +- Runtime 与 Addressables asmdef 编译边界保持稳定。 +- 迁移方案能说明哪些文件先不移动，以及原因。 + +### 2. 最小安装与接入文档 + +目标：让新项目能按文档完成 FlowScope 接入。 + +需求： + +- 编写安装方式： + - 本地 package 路径安装。 + - Git URL 安装。 + - 项目内 Assets 开发模式。 +- 编写最小 bootstrap 示例： + - 创建 Container。 + - 注册 ResourceService 与 AddressablesResourceBackend。 + - 注册 ConfigProvider、SaveService、AudioService、UIManager。 + - 启动 GameFlow。 + - Shutdown 时释放资源和订阅。 +- 明确常见误用： + - 不要把 transient 对象注册成 singleton factory。 + - 不要业务层直接调用 Addressables。 + - 不要用 `UIScreenNavigator` 当完整页面路由。 + - 不要跳过 shutdown cleanup。 + +验收： + +- 文档能独立指导一个空 Unity 项目接入 Runtime。 +- 文档中所有 API 名称与当前代码一致。 +- 示例代码能通过 generated csproj 编译或在 Unity 中编译。 + +### 3. Addressables 真实集成验收 + +目标：补齐 P1 留下的真实 Addressables load/release 验收缺口。 + +需求： + +- 准备最小 Addressables 测试资源或人工验收场景。 +- 验证 `AddressablesResourceBackend.LoadAsync` 成功加载真实资源。 +- 验证 `Release(object asset)` 能释放已加载资源，不产生重复释放异常。 +- 验证资源缺失或类型不匹配时错误可诊断。 +- 如自动化 PlayMode 测试成本过高，先记录可重复的人工验收步骤和结果。 + +验收： + +- 至少有一种可重复验证方式证明 Addressables adapter 不只是编译通过。 +- 验收记录写入 docs，作为 package 发布前 gate。 + +### 4. Editor 诊断工具第一步 + +目标：只做最小检查工具，不做完整编辑器平台。 + +需求： + +- 提供一个 FlowScope 菜单入口或 EditorWindow。 +- 检查项目是否具备必要依赖： + - Addressables package 是否存在。 + - Runtime/Addressables asmdef 是否可见。 + - Samples 是否缺失关键场景或 prefab 引用。 +- 输出可执行的诊断结果，不自动修改项目。 + +验收： + +- 工具可在 Unity Editor 打开。 +- 检查结果不会误报为“已修复”。 +- 没有自动生成或删除用户资源。 + +### 5. Samples 整理 + +目标：让 MainMenuP0 从 P1 验收样例变成 P2 package sample 的候选。 + +需求： + +- 明确 MainMenuP0 是否移动到 `Samples~`，或暂时保留在 `Assets/FlowScope/Samples`。 +- 补充 sample README，说明它覆盖的 Runtime 能力。 +- 保持启动、点击、保存恢复、shutdown cleanup 测试有效。 + +验收： + +- 用户能通过 sample 看懂最小接入方式。 +- PlayMode sample 测试继续全绿。 + +## 暂不进入 P2 的范围 + +- YooAsset / AssetBundle / Resources 多后端完整实现。 +- 云存档、远程配置、热更新配置。 +- 完整 UI 路由、URL 跳转、跨 Feature 页面恢复。 +- AudioMixer、3D 音频、动态音乐系统。 +- 大型资源预算、内存分析和自动修复工具。 +- 完整发布流水线和版本兼容策略。 + +## P2 第一轮建议执行顺序 + +1. 建立 P2 package 结构方案和迁移边界。 +2. 补 Addressables 真实集成验收或人工验收记录。 +3. 编写安装与最小 bootstrap 文档。 +4. 做最小 Editor 诊断入口。 +5. 整理 MainMenuP0 sample 文档和 package sample 候选路径。 + +## 新对话启动提示 + +新对话可以直接使用以下目标： + +```text +根据 docs/requirements/p2-package-editor-requirements.md，先不要直接写代码。请先审查当前 FlowScope P0/P1 基线、package/editor 相关现状和 Addressables 验收缺口，然后输出 P2 第一阶段实施计划。计划必须使用中文，并遵守 AGENTS.md 中的 Git/Plan 规则。 +``` + +## 验收 Gate + +P2 第一阶段完成前必须满足： + +- EditMode 全绿。 +- PlayMode 全绿。 +- generated csproj build 0 error。 +- Addressables adapter 有真实集成验收记录。 +- package/editor 相关新增内容不破坏当前 Assets 内开发模式。 +- 文档能说明安装、接入、验证和已知限制。 diff --git a/docs/reviews/p2-precheck-fifth-review.md b/docs/reviews/p2-precheck-fifth-review.md new file mode 100644 index 0000000..0d0a6e4 --- /dev/null +++ b/docs/reviews/p2-precheck-fifth-review.md @@ -0,0 +1,184 @@ +# FlowScope P2 前第五轮复审意见 + +日期：2026-05-21 +审查对象：当前工作树中 `UIPreloadService` 与 `UIPreloadServiceTests` 的未提交修改 +审查范围：`ReleaseAll()` 后新旧 in-flight preload 隔离、旧任务完成清理、相关 PlayMode 测试、generated csproj 编译状态。 +审查结论：第四轮指出的 P1 gate 已收口；当前没有发现新的 P2 阻塞级问题。剩余主要是 `Release()` 与 `ReleaseAll()` 的 in-flight 语义不完全一致，可作为 P2 前小修或 P2 内 API polish。 + +--- + +## 本轮确认已收口 + +### 1. `ReleaseAll()` 后旧 in-flight 与新 preload 已隔离 + +相关文件： + +- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs` +- `My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs` + +当前 `PreloadAsync` 在发现已有 in-flight 但 generation 过期时，会创建新的 `InFlightPreload`： + +```csharp +if (!_inFlightLoads.TryGetValue(panelType, out inFlightLoad) || + inFlightLoad.ReleaseGeneration != _releaseGeneration) +{ + var attribute = UIManager.GetPanelAttribute(panelType); + path = UIManager.ResolvePath(panelType, attribute); + inFlightLoad = new InFlightPreload(); + inFlightLoad.ReleaseGeneration = _releaseGeneration; + _inFlightLoads[panelType] = inFlightLoad; + startLoad = true; +} +``` + +评价：上一轮指出的“ReleaseAll 后、旧 load 完成前再次 preload 同一 panel 会复用旧任务并最终不缓存”的问题已经修正。 + +--- + +### 2. 旧 in-flight 完成时不会误删新 in-flight + +当前 `LoadAndStoreAsync` 不再直接 `_inFlightLoads.Remove(panelType)`，而是只移除当前任务对应的 in-flight： + +```csharp +private void ForgetInFlightLoad(Type panelType, InFlightPreload inFlightLoad) +{ + if (_inFlightLoads.TryGetValue(panelType, out var current) && + ReferenceEquals(current, inFlightLoad)) + { + _inFlightLoads.Remove(panelType); + } +} +``` + +评价：这个修正是关键的。它避免旧任务完成或失败时把 release 后新建的 in-flight 条目删掉。 + +--- + +### 3. 新增测试覆盖了上一轮缺口 + +新增测试： + +- `PreloadAsync_AfterReleaseAllWhileOldLoadInFlight_StartsNewLoadAndCachesNewHandle` +- `OldInFlightCompletion_AfterReleaseAll_DoesNotRemoveNewInFlight` + +覆盖的真实边界： + +1. 旧 preload 正在加载。 +2. 调用 `ReleaseAll()`。 +3. 立刻再次 preload 同一 panel。 +4. 新 preload 必须触发新的资源加载。 +5. 旧任务完成后只能释放旧 handle，不能污染或删除新任务。 +6. 新任务完成后应缓存新 handle。 + +评价：这两条测试正好覆盖第四轮指出的 P1 风险。 + +--- + +## 当前未发现的 P2 阻塞项 + +从本轮审查范围看，以下上一轮 gate 已经闭环： + +- `ShutdownAsync` best-effort cleanup。 +- `UIPreloadService` 首个 caller cancellation 不再污染其他 waiter。 +- `ReleaseAll/Dispose` 后旧异步任务不再写回缓存。 +- `ReleaseAll` 后重新 preload 同一 panel 不再复用旧 generation 任务。 +- 旧任务完成不会误删新 in-flight。 + +因此，如果 P2 的第一项不是“正式发布资源后端/Addressables 集成质量”，当前 Game Core 主线可以进入 P2。 + +--- + +## 中风险改进 + +### `Release()` 对 in-flight preload 仍不生效 + +风险等级：中 +建议：P2 前可小修；若 P2 首项不扩 UI preload，可放入 P2 内 polish。 + +证据文件： + +- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs` + +当前 `Release()` 只处理已经缓存到 `_handles` 的资源： + +```csharp +if (!_handles.TryGetValue(panelType, out handle)) +{ + return; +} + +_handles.Remove(panelType); +``` + +它不会处理同 panel 的 in-flight preload。结果是： + +1. `PreloadAsync()` 已启动但尚未完成。 +2. 调用 `Release()`。 +3. 因为 `_handles` 里还没有缓存，`Release()` 直接 return。 +4. 旧 preload 完成后仍会缓存 handle。 + +这与当前 `ReleaseAll()` 的语义不完全一致：`ReleaseAll()` 已经通过 `_releaseGeneration++` 明确让所有旧 in-flight 结果失效，而 `Release()` 仍只释放已缓存结果。 + +建议选一种语义并写清楚： + +1. 如果 `Release()` 表示“释放该 panel 的预加载结果”，则应让该 panel 当前 in-flight 失效。 +2. 如果 `Release()` 只表示“释放已缓存 handle，不影响正在进行的 preload”，则建议在 runtime usage guide 明确说明，避免使用者误以为它会取消或丢弃 in-flight 结果。 + +更一致的实现方向： + +- 给 `InFlightPreload` 增加 panel-level generation 或 invalidation flag。 +- `Release()` 在释放 `_handles` 后，同时让当前 panel 的旧 in-flight 结果不能缓存。 +- 补测试：`Release_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle`。 + +--- + +## 低风险改进 + +### Addressables 真实 load/release 验证仍薄 + +当前 `AddressablesResourceBackendTests` 仍主要覆盖 `Name_ReturnsAddressables`。 +这不是本轮 UI preload 修复引入的问题，但仍是包分发或资源后端推广前的验收缺口。 + +建议： + +- 如果 P2 首项是 package/editor tooling，可放到 P2 内第一批验收。 +- 如果 P2 首项是资源后端能力发布，则建议先补最小 Addressables 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 人工场景验收。 +- 本轮审查对象包含未提交工作树修改，不是新 commit。 + +--- + +## P2 Gate 判断 + +当前判断：可以准备进入 P2，但建议先做一次收尾确认。 + +P2 前建议最小动作： + +1. 决定 `Release()` 是否需要与 `ReleaseAll()` 一样使同 panel in-flight 失效。 +2. 如果需要，补一个很小的测试和实现。 +3. 如果暂不处理，在 runtime usage guide 记录 `Release()` 当前只释放已缓存 handle。 +4. 跑一次 Unity Test Runner 或记录无法执行原因。 + +如果你希望严格一点，我建议先修 `Release()` 的 in-flight 语义，再正式开 P2；如果按当前 P2 迭代节奏推进，它已经不是阻塞级问题。 + diff --git a/docs/reviews/p2-precheck-fourth-review.md b/docs/reviews/p2-precheck-fourth-review.md new file mode 100644 index 0000000..bd10fbc --- /dev/null +++ b/docs/reviews/p2-precheck-fourth-review.md @@ -0,0 +1,207 @@ +# FlowScope P2 前第四轮复审意见 + +日期：2026-05-21 +审查对象：`2a88dad 修复预加载释放中的异步写回` +审查范围：`UIPreloadService.ReleaseAll/Dispose` 与 in-flight preload 的生命周期边界、`GameBootstrap` 调用侧、相关 PlayMode 测试、generated csproj 编译状态。 +审查结论：上一轮指出的“释放后异步完成又写回缓存”主问题已修正；但 `ReleaseAll()` 后旧 in-flight 与新 preload 请求仍未隔离，建议 P2 前补掉这个窄边界。 + +--- + +## 本轮确认已收口 + +### 1. 释放后异步写回缓存的问题已基本修正 + +相关文件： + +- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs` +- `My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs` + +当前实现新增： + +- `_releaseGeneration` +- `_disposed` +- `InFlightPreload.ReleaseGeneration` +- `ThrowIfDisposed()` + +`LoadAndStoreAsync()` 完成后只有在以下条件同时满足时才缓存 handle： + +```csharp +if (!_disposed && + inFlightLoad.ReleaseGeneration == _releaseGeneration && + inFlightLoad.WaiterCount > 0) +{ + _handles[panelType] = handle; + shouldCache = true; +} +``` + +对应测试已覆盖： + +- `ReleaseAll_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle` +- `Dispose_WhenLoadInFlight_DoesNotCacheAndDisposesLoadedHandle` +- `PreloadAsync_AfterDispose_ThrowsObjectDisposedException` + +评价：上一轮指出的 “ReleaseAll/Dispose 后，旧异步任务完成又把资源写回 `_handles`” 已经被 generation/disposed gate 挡住。 + +--- + +### 2. Dispose 后拒绝新 preload 请求 + +当前 `PreloadAsync` 在 lock 内调用： + +```csharp +ThrowIfDisposed(); +``` + +并且 `IsPreloaded` / `TryGetPreloadedHandle` 在 `_disposed` 后返回 false。 + +评价：`Dispose` 后服务不再接受 preload，语义清楚，和 Container / ResourceEntry 这类对象的 disposed 行为更一致。 + +--- + +### 3. GameBootstrap 调用侧仍保持 best-effort cleanup + +相关文件： + +- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs` + +`ShutdownAsync` 仍会在保存、GameFlow shutdown、preload release 三步里收集异常，并尽量执行完整清理。 + +评价：上一轮 shutdown gate 未被本轮改动破坏。 + +--- + +## 仍建议 P2 前修正的问题 + +### P1. `ReleaseAll()` 后、旧 in-flight 完成前，新 preload 会复用旧任务并最终不缓存 + +风险等级：高 +建议：进入 P2 前修正。 + +证据文件： + +- `My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs` + +当前 `ReleaseAll()` 只递增 generation 并清理已缓存 handle： + +```csharp +lock (_gate) +{ + _releaseGeneration++; + handles = new List>(_handles.Values); + _handles.Clear(); +} +``` + +但它不会移除或隔离 `_inFlightLoads`： + +```csharp +private readonly Dictionary _inFlightLoads = new(); +``` + +问题场景： + +1. 调用 `PreloadAsync()`，创建旧 `InFlightPreload`，其 `ReleaseGeneration = 0`。 +2. 底层资源仍在加载。 +3. 调用 `ReleaseAll()`，`_releaseGeneration` 变为 1。 +4. 在旧加载完成前，再次调用 `PreloadAsync()`。 +5. 因为 `_inFlightLoads` 里仍有旧 in-flight，新请求会复用旧任务，并把 `WaiterCount++`。 +6. 旧加载完成后，`ReleaseGeneration != _releaseGeneration`，因此 handle 被 dispose，不缓存。 +7. 第二个 preload 请求 await 成功返回，但 `IsPreloaded() == false`。 + +这会造成 API 使用者视角的语义不一致：`PreloadAsync` 成功完成，但资源并没有处于 preloaded 状态。 +如果 P2 后 UI preload 被当成可复用框架能力，这个边界会在“释放后立即重新预热同一 UI”的场景里变成隐性 bug。 + +建议修法： + +1. `ReleaseAll()` 应让 release 前的 in-flight 与 release 后的新请求隔离。 +2. 可选实现方向： + - `ReleaseAll()` 清理 `_inFlightLoads`，但 `LoadAndStoreAsync()` 完成时必须用 `ReferenceEquals(current, inFlightLoad)` 移除，避免旧任务误删新任务。 + - 或保留旧 in-flight，但 `PreloadAsync` 发现 `inFlightLoad.ReleaseGeneration != _releaseGeneration` 时，不复用旧任务，而是创建新 generation 的 in-flight。 +3. `LoadAndStoreAsync()` 里的 `_inFlightLoads.Remove(panelType)` 建议改成“只移除当前 in-flight”： + +```csharp +if (_inFlightLoads.TryGetValue(panelType, out var current) && + ReferenceEquals(current, inFlightLoad)) +{ + _inFlightLoads.Remove(panelType); +} +``` + +否则一旦允许 release 后创建新 in-flight，旧任务完成时可能误删新任务。 + +建议补测： + +- `PreloadAsync_AfterReleaseAllWhileOldLoadInFlight_StartsNewLoadAndCachesNewHandle` +- `OldInFlightCompletion_AfterReleaseAll_DoesNotRemoveNewInFlight` + +--- + +## 中低风险观察 + +### 1. `ReleaseAll_WhenLoadInFlight` 当前测试只覆盖“不缓存旧 handle” + +现有测试验证： + +- 旧 load 完成后 handle 被 dispose。 +- `IsPreloaded` 为 false。 + +但没有覆盖 release 后立即再次 preload 的重入语义。 +这正是当前剩余风险所在。 + +### 2. `PreloadAsync` 在 `ReleaseAll()` 后等待旧任务成功返回的语义仍偏模糊 + +如果调用者在 `ReleaseAll()` 前已经开始等待旧 task，旧 task 最终成功返回但资源被 dispose、不缓存。 +这在 shutdown 场景可以接受，因为 release 代表放弃预热结果；但在普通运行时场景，最好通过文档或测试确认这是刻意语义。 + +若希望更严格，可以考虑让被 release generation 淘汰的等待者收到取消或特定异常。 +不过这会扩大改动范围，不一定是 P2 前最小必要项。 + +--- + +## 仍可后置的风险 + +### Addressables 真实 load/release 验证仍薄 + +本轮没有改变 Addressables 适配层。 +generated csproj 能编译，但真实 Addressables 资源加载、释放、失败诊断仍缺少强验证。 + +建议保留为 P2 内第一批验收项，除非 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 generated csproj 常见引用冲突 warning。 + +说明： + +- 本轮未执行 Unity Test Runner。 +- 本轮未做 MainMenuP0 人工场景验收。 + +--- + +## P2 Gate 判断 + +当前建议：仍然暂缓进入 P2，先修一个更窄的 UI preload 重入问题。 + +P2 前最小清单： + +1. 修复 `ReleaseAll()` 后旧 in-flight 与新 `PreloadAsync` 请求的隔离。 +2. 将 `_inFlightLoads.Remove(panelType)` 改为只移除当前 in-flight，避免旧任务误删新任务。 +3. 补 release 后立即重新 preload 同一 panel 的 PlayMode 测试。 +4. 保持 EditMode / PlayMode generated csproj build 0 error。 + +完成这项后，P2 gate 基本可以认为通过；剩余 Addressables 真实集成验收可以作为 P2 内第一项或 package 发布前验收项。 +