Files
FlowScope/docs/reviews/p1-architecture-audit-before-p2.md
2026-05-21 11:28:56 +08:00

424 lines
16 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
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.
# FlowScope P1 后架构审核与 P2 前置整改清单
日期2026-05-21
范围FlowScope Game Core 当前 P0/P1 实现、P0/P1 需求文档、P1 completion report、runtime usage guide。
目的:作为后续执行 session 的整改依据。本文只定义问题、风险、建议和验收标准,不直接展开 P2 实施。
---
## 结论
当前 FlowScope 已经形成 P0/P1 轻量 Unity 游戏框架骨架,但不建议直接进入 P2。
核心原因不是功能数量不足,而是若干 P0/P1 内核语义还没有稳定Container 生命周期、Feature scope、GameFlow 与 Bootstrap 职责、样例启动退出模型、资源/UI 取消与预加载边界、文档状态一致性。P2 的包分发、编辑器工具和跨项目复用会固化这些 API若此时推进后续修正成本会显著升高。
建议先执行“P2 前最小整改清单”,确认内核语义稳定后,再进入 P2。
---
## 整改进展2026-05-21
已执行第一轮 P2 前最小整改:
- Container 已明确 `RegisterSingletonFactory` / `RegisterScoped` / `RegisterTransient` 三类生命周期,`RegisterFactory` 仅保留为旧式 lazy singleton 兼容入口。
- Feature 示例和测试改为 transient 注册,`GameFlow``A -> B -> A` 重入会创建新的 Feature 实例Feature 释放归属 Feature scope。
- MainMenuP0 `GameBootstrap` 已移除 `async void Awake` / `async void OnApplicationQuit` 的直接 await 模式,启动任务通过 `StartupTask` 可观察,退出异常集中记录。
- ResourceService 已补共享加载取消边界测试,覆盖单个等待者取消、全部等待者取消后完成即释放且不缓存。
- UIPreloadService 已补同 Panel 并发预加载合并测试,并实现 in-flight load 复用。
- P1 completion report、runtime usage guide、framework review、P1 需求验收状态已同步当前代码状态。
仍未完成:
- Unity Test Runner 全量 EditMode / PlayMode 验收记录。
- Addressables 真实 load/release 最小集成测试或人工验收步骤。
- `UIManager` 运行时 `DestroyImmediate` 风险收口。
- Config/Save DTO 与 AOT/linker 限制文档或验证。
---
## 最新验证证据
本次审核阶段只做代码阅读和轻量编译验证,没有修改运行时代码。
已执行:
```powershell
dotnet build "My project\FlowScope.Runtime.csproj" --no-restore
dotnet build "My project\FlowScope.Tests.EditMode.csproj" --no-restore
dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore
```
结果:
- 三个 `dotnet build` 均为 0 error。
- 存在 Unity 生成项目的引用版本冲突 warning主要来自 `System.Net.Http``System.Security.Cryptography.Algorithms``System.ComponentModel.Annotations`
- 未在本次审核中重新执行 Unity Test Runner因此不能用本文替代 Unity EditMode / PlayMode 测试结果。
---
## 阻塞级 / 高风险问题
### 1. Container 工厂生命周期与 Feature scope 语义冲突
风险等级:阻塞级
建议P2 前必须修。
证据:
- `My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs`
- `ForFactory` 创建的 registration 会缓存 `_instance`
- `Resolve()``_created == true` 后直接返回旧实例。
- 工厂创建的 `IDisposable``_owner.TrackOwnedDisposable` 跟踪。
- `My project/Assets/FlowScope/Runtime/Container/Container.cs`
- 子 scope 找不到注册时会递归返回父 scope 的 registration。
- `My project/Assets/FlowScope/Runtime/Flow/GameFlow.cs`
- `CreateFeature<TFeature>` 创建 feature scope 后调用 `scope.Resolve<TFeature>()`
- `My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs`
- `MainMenuFeature` 注册在 root`_root.RegisterFactory(_ => new MainMenuFeature(...))`
真实行为:
- `RegisterFactory` 名义上像“工厂”,实际是“懒加载单例”。
- root 注册的 Feature 被 feature scope 解析时,仍使用 root registration。
- 同一个 Feature 类型如果再次进入,可能复用已经 `Dispose` 过的旧实例。
- 由父 scope registration 创建的对象由父 scope 持有释放,削弱 Feature scope 的生命周期隔离。
影响:
- 破坏 P0 文档中的“GameFlow 创建 Feature scopeFeature 临时对象随 FeatureContext 释放”。
- Feature 之间可能共享不该共享的实例状态。
- P2 包化后,使用者会照着 `RegisterFactory` 注册 Feature/ViewModel/临时对象,埋下复用已释放对象的隐性 bug。
整改建议:
1. 明确 Container 生命周期模型,至少区分:
- singleton/root service
- scoped/feature-owned service
- transient/new instance per resolve
2. 调整 API 命名,避免 `RegisterFactory` 继续误导:
- 方案 A保留现有缓存语义但改名或补充 `RegisterSingletonFactory`,另加 `RegisterTransient`
- 方案 B`RegisterFactory` 真正每次 resolve 创建新实例,再另加 singleton API。
3. 明确父注册在子 scope 下解析时的创建者与释放者:
- 若是 singleton应由注册所在 scope 持有。
- 若是 scoped/transient应由请求 scope 持有。
4. GameFlow 创建 Feature 时应确保每次进入得到新 Feature 实例,不能复用已 Dispose 实例。
最低验收:
- 新增测试:`A -> B -> A`,第二次进入 `A` 必须是新实例。
- 新增测试root 注册工厂child scope resolve 时生命周期归属符合文档。
- 新增测试Feature scope dispose 后,不影响 root singleton但会释放 scoped/transient 对象。
- 新增测试:已 Dispose 的 Feature 不会在再次切换时被复用。
- 更新 `docs/guides/flowscope-runtime-usage.md` 的 Container 注册规则。
---
### 2. `RegisterFactory` API 名称和实现语义不一致
风险等级:高
建议P2 前必须修或正式写入限制。
证据:
- P0 文档把 `RegisterFactory<T>(Func<Container, T> factory)` 作为显式工厂注册。
- 实现中该 factory 只会在第一次 Resolve 时执行,后续返回缓存实例。
影响:
- 使用者自然会认为 factory 是“每次创建”或至少“不缓存业务对象”。
- 目前它更接近 lazy singleton。
- 若 P2 后作为公开包 API 发布,后续纠正会是破坏性变更。
整改建议:
- 不要只补文档,应优先调整 API 语义或新增明确 API。
- 如果为了 P1 兼容暂不破坏旧接口,也要让旧接口行为在文档中被明确标注,并禁止用于 Feature/ViewModel/短生命周期对象。
最低验收:
- Container 测试中明确覆盖 singleton/transient/scoped 三类行为。
- 文档中出现清晰示例root service 如何注册Feature 如何注册,临时对象如何注册。
---
### 3. GameFlow 与 Bootstrap 的生命周期职责边界不一致
风险等级:高
建议P2 前必须定案。
证据:
- P0 文档描述 GameFlow 职责包括启动时加载或创建 Data、关闭时保存必要数据。
- 当前 `GameFlow.StartupAsync` 固定调用 `IConfigProvider.LoadAllAsync`
- 当前保存 `PlayerData` 在样例 `GameBootstrap.ShutdownAsync` 中执行,而不是 GameFlow。
真实架构:
- 当前 GameFlow 更像 Feature 生命周期编排器。
- Bootstrap 才是真正的应用装配、数据加载、存档保存入口。
- 但 GameFlow 又硬编码了配置加载,导致职责介于“纯编排器”和“应用启动流程所有者”之间。
影响:
- 使用者不清楚 Data/Save 应该挂在 GameFlow 还是 Bootstrap。
- P2 做模板或包分发时,启动流程样板会变得含混。
- 后续如果加入多 Feature、预加载、场景切换会更难判断谁拥有启动任务列表。
整改建议:
二选一:
1. GameFlow 退回纯 Feature 编排器:
- 移除或策略化 `IConfigProvider.LoadAllAsync`
- Config/Save/Data 由 Bootstrap 或 AppStartupPipeline 负责。
2. GameFlow 正式成为 AppFlow
- 引入明确启动任务/保存任务策略。
- 将 Data load/save 纳入可配置流程,而不是只硬编码 Config。
最低验收:
- 文档和代码一致说明Config 加载、Save 加载、Save 写回分别由谁负责。
- 样例不再与主文档互相矛盾。
- GameFlow 测试覆盖职责定案后的启动/关闭行为。
---
### 4. 样例 Bootstrap 使用 `async void`,容易被业务照抄
风险等级:高
建议P2 前修。
证据:
- `GameBootstrap.Awake()``async void` 并直接 await `StartupAsync`
- `GameBootstrap.OnApplicationQuit()``async void` 并直接 await `ShutdownAsync`
影响:
- 启动异常可能成为未观察异常或只表现为 Unity 日志。
- 退出时异步保存和释放没有统一错误处理入口。
- 这是官方样例,使用者很可能照抄。
整改建议:
- Bootstrap 应有可观察的启动任务和错误处理策略。
- `Awake` 可只创建 lifetime token`Start` 或显式 `RunAsync` 负责启动。
- 异常应写入明确日志,并让测试能够断言。
- 退出保存应考虑重复调用、取消、异常 best-effort 规则。
最低验收:
- 样例启动异常可被测试捕获或至少被统一日志记录。
- 重复 Shutdown 不重复释放、不重复保存或语义明确。
- OnDestroy / OnApplicationQuit 顺序不会造成 token 已释放后继续使用。
---
## 中风险问题
### 5. UI 预加载并发和释放边界偏薄
风险等级:中
证据:
- `UIPreloadService.PreloadAsync<TPanel>` 没有同类型并发去重。
- `UIManager` 对预加载 handle 使用空释放句柄,实际释放依赖 `UIPreloadService.Release` / `ReleaseAll`
影响:
- 两个并发预加载可能重复加载并覆盖 handle。
- UI 实例关闭不释放预加载 handle 是合理设计,但需要更强文档和测试防误用。
整改建议:
-`UIPreloadService` 增加同 Panel 类型的 in-flight load 合并。
- 增加测试:并发 Preload 只触发一次底层加载。
- 增加测试:已打开预加载 Panel 时提前 Release 的行为被定义清楚。
---
### 6. `UIScreenNavigator` 是轻量栈,不应被视为完整路由
风险等级:中
证据:
- `UIScreenNavigator` 只保存 `Action` close stack。
- 外部 `UIManager.Close<TPanel>` / `CloseLayer` / `CloseAll` 不会同步导航栈。
影响:
- 外部关闭后Navigator 内部可能保留已经关闭的历史 action。
- P1 可接受,但 P2 文档不能把它包装成完整页面路由。
整改建议:
- 文档明确它只管理自己 push 的页面。
- 如果 P2 要做完整导航,应重新设计 route/state而不是扩这个 action stack。
---
### 7. 资源共享加载取消语义还需要补边界测试
风险等级:中
证据:
- `ResourceService.LoadEntryAsync` 调用 backend 时传入 `CancellationToken.None`
- 调用者取消只取消等待,不取消共享底层加载。
- 这是合理设计,但行为复杂。
影响:
- “所有等待者取消后,底层加载完成再释放”这类边界如果没测,容易出现泄漏、重复 release 或未观察异常。
整改建议:
- 补测试:
- 多等待者中一个取消,另一个成功拿到 handle。
- 所有等待者取消,底层随后成功,资源被释放且不留 completed cache。
- 所有等待者取消,底层随后失败,没有未观察异常或残留 `_loads`
---
### 8. Addressables 适配层测试过薄
风险等级:中
证据:
- `AddressablesResourceBackendTests` 当前只测 `Name_ReturnsAddressables`
- PlayMode 编译已能通过,但不等于真实 Addressables load/release 验证通过。
整改建议:
- 增加最小 Addressables 测试资源。
- 验证 `AddressablesResourceBackend.LoadAsync<T>` 成功加载、取消、失败、Release。
- 如果测试资源维护成本高,至少增加一条文档化的人工验收步骤。
---
### 9. 自研 JSON / 反射 / AOT 风险仍未收口
风险等级:中
证据:
- `ReactivePropertyJsonConverter` 手写 JSON parse/write。
- `JsonSaveSerializer` 使用 `Activator.CreateInstance`
- Config mapping 使用反射 `MakeGenericMethod` / `Invoke`
影响:
- P1 阶段可接受。
- P2 包化和移动端 IL2CPP 下,需要提前验证或明确限制。
整改建议:
- P2 前至少写清 DTO 限制:
- 必须有无参构造。
- 支持字段/属性类型范围。
- 不支持复杂 polymorphism、Dictionary、UnityEngine.Object 引用等。
- 增加 IL2CPP/linker 验证计划,或改用成熟 JSON 库并保留 R3 适配层。
---
## 文档与实现不一致
### A. P1 completion report 已过时
文档位置:
- `docs/reviews/p1-completion-report.md`
- `docs/requirements/p1-production-hardening.md`
- `docs/reviews/flowscope-framework-review.md`
不一致点:
- 文档仍描述 PlayMode 完整编译因 Addressables 未解析失败。
- 当前 `packages-lock.json` 已包含 `com.unity.addressables`
- 本次审核运行 `dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore` 为 0 error。
建议:
- 更新完成报告,把“旧阻塞已解除”和“仍需 Unity Test Runner/人工验收确认”分开写。
- 不要继续保留“Addressables 尚未解析导致 PlayMode 编译失败”的当前状态描述。
### B. AudioService 旧风险描述需要更新
文档位置:
- `docs/reviews/flowscope-framework-review.md`
- `docs/reviews/p1-completion-report.md`
不一致点:
- 旧评审提到 AudioService 同步阻塞异步加载。
- 当前实现已经提供 `PlayBgmAsync` / `PlaySfxAsync`,同步方法只允许读取 `ICompletedResourceService` 已完成资源,不再 `.GetAwaiter().GetResult()` 阻塞。
建议:
- 更新文档:旧同步阻塞风险已降低。
- 新风险应改为:同步 API 只适合预加载完成资源,使用者必须理解并处理异常。
### C. GameFlow 职责文档与实现不一致
文档位置:
- `docs/requirements/p0-requirements-set.md`
- `docs/requirements/p0-gameflow.md`
- `docs/guides/flowscope-runtime-usage.md`
不一致点:
- 文档把 GameFlow 写成全局生命周期所有者。
- 实现中保存、Data 加载、预加载释放主要由 Bootstrap 负责。
- GameFlow 只硬编码 Config 加载和 Feature 生命周期。
建议:
- 先做架构决策,再同步文档。
---
## P2 前最小整改清单
必须先修:
1. 修正或定案 Container 生命周期语义。
2. 保证 Feature 每次进入都是新实例,且 Feature scope 拥有自己的短生命周期对象。
3. 明确 GameFlow 与 Bootstrap 职责边界,并让代码和文档一致。
4. 修正样例 Bootstrap 的 `async void` 启动/退出风险。
5. 更新 P1 completion report / runtime usage guide / framework review 中的过时状态。
建议同步补测:
1. Containersingleton / scoped / transient 行为。
2. GameFlow`A -> B -> A` 重入。
3. ResourceService共享加载取消边界。
4. UIPreloadService并发预加载与释放语义。
5. Addressables真实 load/release 最小 PlayMode 验证。
完成以上后再评估 P2。
---
## 执行 session 建议顺序
1. 先只改 Container 和 GameFlow 相关语义,不碰 P2 包分发。
2. 跑 EditMode 测试,确认核心生命周期稳定。
3. 再改样例 Bootstrap让 MainMenuP0 继续作为真实使用样板。
4. 跑 PlayMode 测试,确认 UI、样例、资源释放没有退化。
5. 最后更新文档,确保 report 不再保留旧阻塞状态。
---
## 禁止事项
- 不要在本轮顺手进入 P2。
- 不要扩大到 Luban、YooAsset、云存档、AudioMixer、完整 UI 路由。
- 不要为了修测试而弱化 Feature scope 或资源释放语义。
- 不要只改文档掩盖 Container 生命周期问题。
- 不要把样例变成特殊路径;样例应继续代表推荐使用方式。