From 558bc919efe983ace0d9aa555a483ca27f9bd86e Mon Sep 17 00:00:00 2001 From: "JSD\\13999" <1399945104@qq.com> Date: Thu, 21 May 2026 11:28:56 +0800 Subject: [PATCH] =?UTF-8?q?=E5=AE=8C=E6=88=90=20P2=20=E5=89=8D=E6=9E=B6?= =?UTF-8?q?=E6=9E=84=E6=95=B4=E6=94=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../FlowScope/Runtime/Container/Container.cs | 46 +- .../Container/ContainerRegistration.cs | 93 +++- .../Assets/FlowScope/Runtime/Flow/GameFlow.cs | 25 +- .../FlowScope/Runtime/UI/UIPreloadService.cs | 38 +- .../MainMenuP0/Scripts/GameBootstrap.cs | 44 +- .../EditMode/Container/ContainerTests.cs | 86 ++++ .../Tests/EditMode/Flow/GameFlowTests.cs | 30 +- .../Resources/ResourceServiceBackendTests.cs | 44 ++ .../Tests/PlayMode/Samples/MainMenuP0Tests.cs | 20 + .../PlayMode/UI/UIPreloadServiceTests.cs | 69 +++ docs/guides/flowscope-runtime-usage.md | 8 +- docs/requirements/p1-production-hardening.md | 4 +- docs/reviews/flowscope-framework-review.md | 46 +- .../p1-architecture-audit-before-p2.md | 423 ++++++++++++++++++ docs/reviews/p1-completion-report.md | 13 +- 15 files changed, 895 insertions(+), 94 deletions(-) create mode 100644 docs/reviews/p1-architecture-audit-before-p2.md diff --git a/My project/Assets/FlowScope/Runtime/Container/Container.cs b/My project/Assets/FlowScope/Runtime/Container/Container.cs index c648e25..e067c4e 100644 --- a/My project/Assets/FlowScope/Runtime/Container/Container.cs +++ b/My project/Assets/FlowScope/Runtime/Container/Container.cs @@ -37,6 +37,11 @@ namespace FlowScope.Container } public void RegisterFactory(Func factory) + { + RegisterSingletonFactory(factory); + } + + public void RegisterSingletonFactory(Func factory) { ThrowIfDisposed(); @@ -45,7 +50,31 @@ namespace FlowScope.Container throw new ArgumentNullException(nameof(factory)); } - RegisterLocal(typeof(T), ContainerRegistration.ForFactory(this, c => factory(c))); + RegisterLocal(typeof(T), ContainerRegistration.ForSingletonFactory(this, c => factory(c))); + } + + public void RegisterScoped(Func factory) + { + ThrowIfDisposed(); + + if (factory == null) + { + throw new ArgumentNullException(nameof(factory)); + } + + RegisterLocal(typeof(T), ContainerRegistration.ForScopedFactory(this, c => factory(c))); + } + + public void RegisterTransient(Func factory) + { + ThrowIfDisposed(); + + if (factory == null) + { + throw new ArgumentNullException(nameof(factory)); + } + + RegisterLocal(typeof(T), ContainerRegistration.ForTransientFactory(this, c => factory(c))); } public void RegisterType() where TImplementation : TInterface @@ -121,6 +150,7 @@ namespace FlowScope.Container _children.Clear(); _ownedDisposables.Clear(); + ForgetScopedInstancesFor(this); _registrations.Clear(); if (_parent != null) @@ -160,7 +190,7 @@ namespace FlowScope.Container throw new ArgumentNullException(nameof(factory)); } - RegisterLocal(serviceType, ContainerRegistration.ForFactory(this, factory)); + RegisterLocal(serviceType, ContainerRegistration.ForTransientFactory(this, factory)); } internal void TrackOwnedDisposable(IDisposable disposable) @@ -209,7 +239,7 @@ namespace FlowScope.Container _resolutionStack.Push(serviceType); try { - return registration.Resolve(); + return registration.Resolve(this); } finally { @@ -236,6 +266,16 @@ namespace FlowScope.Container return false; } + private void ForgetScopedInstancesFor(Container scope) + { + foreach (var registration in _registrations.Values) + { + registration.ForgetScope(scope); + } + + _parent?.ForgetScopedInstancesFor(scope); + } + private void ThrowIfDisposed() { if (_disposed) diff --git a/My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs b/My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs index 0ad4df9..c414d22 100644 --- a/My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs +++ b/My project/Assets/FlowScope/Runtime/Container/ContainerRegistration.cs @@ -1,50 +1,117 @@ using System; +using System.Collections.Generic; namespace FlowScope.Container { internal sealed class ContainerRegistration { + private enum Lifetime + { + Instance, + Singleton, + Scoped, + Transient + } + private readonly Container _owner; private readonly Func _factory; - private readonly bool _ownsCreatedInstance; + private readonly Lifetime _lifetime; + private readonly Dictionary _scopedInstances; private object _instance; private bool _created; - private ContainerRegistration(Container owner, Func factory, object instance, bool created, bool ownsCreatedInstance) + private ContainerRegistration( + Container owner, + Func factory, + object instance, + bool created, + Lifetime lifetime) { _owner = owner; _factory = factory; _instance = instance; _created = created; - _ownsCreatedInstance = ownsCreatedInstance; + _lifetime = lifetime; + _scopedInstances = lifetime == Lifetime.Scoped + ? new Dictionary() + : null; } public static ContainerRegistration ForInstance(object instance) { - return new ContainerRegistration(null, null, instance, true, false); + return new ContainerRegistration(null, null, instance, true, Lifetime.Instance); } - public static ContainerRegistration ForFactory(Container owner, Func factory) + public static ContainerRegistration ForSingletonFactory(Container owner, Func factory) { - return new ContainerRegistration(owner, factory, null, false, true); + return new ContainerRegistration(owner, factory, null, false, Lifetime.Singleton); } - public object Resolve() + public static ContainerRegistration ForScopedFactory(Container owner, Func factory) { - if (_created) + return new ContainerRegistration(owner, factory, null, false, Lifetime.Scoped); + } + + public static ContainerRegistration ForTransientFactory(Container owner, Func factory) + { + return new ContainerRegistration(owner, factory, null, false, Lifetime.Transient); + } + + public object Resolve(Container requestScope) + { + switch (_lifetime) { - return _instance; + case Lifetime.Instance: + return _instance; + case Lifetime.Singleton: + return ResolveSingleton(); + case Lifetime.Scoped: + return ResolveScoped(requestScope); + case Lifetime.Transient: + return CreateOwnedInstance(requestScope); + default: + throw new InvalidOperationException($"Unsupported container lifetime {_lifetime}."); } + } - _instance = _factory(_owner); - _created = true; + public void ForgetScope(Container scope) + { + _scopedInstances?.Remove(scope); + } - if (_ownsCreatedInstance && _instance is IDisposable disposable) + private object ResolveSingleton() + { + if (!_created) { - _owner.TrackOwnedDisposable(disposable); + _instance = CreateOwnedInstance(_owner); + _created = true; } return _instance; } + + private object ResolveScoped(Container requestScope) + { + if (_scopedInstances.TryGetValue(requestScope, out var instance)) + { + return instance; + } + + instance = CreateOwnedInstance(requestScope); + _scopedInstances.Add(requestScope, instance); + return instance; + } + + private object CreateOwnedInstance(Container owner) + { + var instance = _factory(owner); + + if (instance is IDisposable disposable) + { + owner.TrackOwnedDisposable(disposable); + } + + return instance; + } } } diff --git a/My project/Assets/FlowScope/Runtime/Flow/GameFlow.cs b/My project/Assets/FlowScope/Runtime/Flow/GameFlow.cs index f1883b0..d0b2eb4 100644 --- a/My project/Assets/FlowScope/Runtime/Flow/GameFlow.cs +++ b/My project/Assets/FlowScope/Runtime/Flow/GameFlow.cs @@ -44,7 +44,6 @@ namespace FlowScope.Flow } catch { - BestEffortDisposeFeature(feature); BestEffortDisposeContext(context); _activeFeature = null; _activeContext = null; @@ -79,7 +78,6 @@ namespace FlowScope.Flow } catch { - BestEffortDisposeFeature(nextFeature); BestEffortDisposeContext(nextContext); _activeFeature = null; _activeContext = null; @@ -170,30 +168,11 @@ namespace FlowScope.Flow { Debug.LogError(exception); } - - BestEffortDisposeFeature(feature); } BestEffortDisposeContext(context); } - private void BestEffortDisposeFeature(IFeature feature) - { - if (feature == null) - { - return; - } - - try - { - feature.Dispose(); - } - catch (Exception exception) - { - Debug.LogError(exception); - } - } - private void BestEffortDisposeContext(FeatureContext context) { if (context == null) @@ -212,7 +191,7 @@ namespace FlowScope.Flow try { - context.Resources?.Dispose(); + context.Scope?.Dispose(); } catch (Exception exception) { @@ -221,7 +200,7 @@ namespace FlowScope.Flow try { - context.Scope?.Dispose(); + context.Resources?.Dispose(); } catch (Exception exception) { diff --git a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs index 4dd6f01..ebe970b 100644 --- a/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs +++ b/My project/Assets/FlowScope/Runtime/UI/UIPreloadService.cs @@ -11,6 +11,7 @@ namespace FlowScope.UI { private readonly IResourceService _resources; private readonly Dictionary> _handles = new(); + private readonly Dictionary _inFlightLoads = new(); public UIPreloadService(IResourceService resources) { @@ -27,16 +28,24 @@ namespace FlowScope.UI return; } - var attribute = UIManager.GetPanelAttribute(panelType); - var path = UIManager.ResolvePath(panelType, attribute); - var handle = await _resources.LoadAsync(path, cancellationToken); - if (handle == null || handle.Asset == null) + if (_inFlightLoads.TryGetValue(panelType, out var inFlightLoad)) { - handle?.Dispose(); - throw new InvalidOperationException($"UI panel resource is missing: {path}"); + await inFlightLoad; + return; } - _handles[panelType] = handle; + var attribute = UIManager.GetPanelAttribute(panelType); + var path = UIManager.ResolvePath(panelType, attribute); + var loadTask = LoadAndStoreAsync(panelType, path, cancellationToken); + _inFlightLoads.Add(panelType, loadTask); + try + { + await loadTask; + } + finally + { + _inFlightLoads.Remove(panelType); + } } public bool IsPreloaded() @@ -85,5 +94,20 @@ namespace FlowScope.UI { ReleaseAll(); } + + private async Task LoadAndStoreAsync( + Type panelType, + string path, + CancellationToken cancellationToken) + { + var handle = await _resources.LoadAsync(path, cancellationToken); + if (handle == null || handle.Asset == null) + { + handle?.Dispose(); + throw new InvalidOperationException($"UI panel resource is missing: {path}"); + } + + _handles[panelType] = handle; + } } } diff --git a/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs b/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs index 9ab9e7c..1df42ea 100644 --- a/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs +++ b/My project/Assets/FlowScope/Samples/MainMenuP0/Scripts/GameBootstrap.cs @@ -30,15 +30,17 @@ namespace FlowScope.Samples.MainMenuP0 private PlayerData _playerData; private CancellationTokenSource _lifetime; private bool _started; + private bool _shutdownStarted; private bool _shutdown; public int ReleasedResourceCount => _sampleBackend?.ReleaseCount ?? 0; public bool IsShutdown => _shutdown; + public Task StartupTask { get; private set; } - private async void Awake() + private void Awake() { _lifetime = new CancellationTokenSource(); - await StartupAsync(_lifetime.Token); + StartupTask = RunStartupWithLoggingAsync(_lifetime.Token); } public async Task StartupAsync(CancellationToken cancellationToken) @@ -97,13 +99,20 @@ namespace FlowScope.Samples.MainMenuP0 _root.RegisterInstance(_preloadService); _root.RegisterInstance(screenNavigator); _root.RegisterInstance(_playerData); - _root.RegisterFactory(_ => new MainMenuFeature(screenNavigator, _playerData)); + _root.RegisterTransient(_ => new MainMenuFeature(screenNavigator, _playerData)); await _gameFlow.StartupAsync(_root, cancellationToken); } public async Task ShutdownAsync(CancellationToken cancellationToken) { + if (_shutdownStarted) + { + return; + } + + _shutdownStarted = true; + if (_saveService != null && _playerData != null) { await _saveService.SaveAsync(SaveKey, _playerData, cancellationToken); @@ -118,10 +127,10 @@ namespace FlowScope.Samples.MainMenuP0 _shutdown = true; } - private async void OnApplicationQuit() + private void OnApplicationQuit() { _lifetime?.Cancel(); - await ShutdownAsync(CancellationToken.None); + _ = RunShutdownWithLoggingAsync(CancellationToken.None); } private void OnDestroy() @@ -130,6 +139,31 @@ namespace FlowScope.Samples.MainMenuP0 _lifetime?.Dispose(); } + private async Task RunStartupWithLoggingAsync(CancellationToken cancellationToken) + { + try + { + await StartupAsync(cancellationToken); + } + catch (Exception exception) + { + Debug.LogException(exception); + throw; + } + } + + private async Task RunShutdownWithLoggingAsync(CancellationToken cancellationToken) + { + try + { + await ShutdownAsync(cancellationToken); + } + catch (Exception exception) + { + Debug.LogException(exception); + } + } + private Canvas ResolveHudCanvas() { if (hudCanvas != null) diff --git a/My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs b/My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs index 7b1e492..fc0c271 100644 --- a/My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs +++ b/My project/Assets/FlowScope/Tests/EditMode/Container/ContainerTests.cs @@ -44,6 +44,71 @@ namespace FlowScope.Tests.EditMode.Container CollectionAssert.AreEqual(new[] { "second", "first" }, disposeOrder); } + [Test] + public void RegisterSingletonFactory_Resolve_CreatesOnceAndIsOwnedByRegistrationScope() + { + var disposeOrder = new List(); + var parent = new FlowScope.Container.Container(); + parent.RegisterSingletonFactory(_ => new DisposableNamedService("singleton", disposeOrder)); + var child = parent.CreateScope(); + + var first = child.Resolve(); + var second = child.Resolve(); + + Assert.AreSame(first, second); + + child.Dispose(); + + CollectionAssert.IsEmpty(disposeOrder); + + parent.Dispose(); + + CollectionAssert.AreEqual(new[] { "singleton" }, disposeOrder); + } + + [Test] + public void RegisterTransient_Resolve_CreatesNewInstanceOwnedByRequestScope() + { + var disposeOrder = new List(); + var parent = new FlowScope.Container.Container(); + parent.RegisterTransient(_ => new DisposableNamedService("transient", disposeOrder)); + var child = parent.CreateScope(); + + var first = child.Resolve(); + var second = child.Resolve(); + + Assert.AreNotSame(first, second); + + child.Dispose(); + + CollectionAssert.AreEqual(new[] { "transient", "transient" }, disposeOrder); + } + + [Test] + public void RegisterScoped_Resolve_CreatesOneInstancePerRequestScope() + { + var disposeOrder = new List(); + var parent = new FlowScope.Container.Container(); + parent.RegisterScoped(_ => new DisposableNamedService("scoped", disposeOrder)); + var firstChild = parent.CreateScope(); + var secondChild = parent.CreateScope(); + + var firstA = firstChild.Resolve(); + var firstB = firstChild.Resolve(); + var second = secondChild.Resolve(); + + Assert.AreSame(firstA, firstB); + Assert.AreNotSame(firstA, second); + + firstChild.Dispose(); + + CollectionAssert.AreEqual(new[] { "scoped" }, disposeOrder); + + secondChild.Dispose(); + + CollectionAssert.AreEqual(new[] { "scoped", "scoped" }, disposeOrder); + } + [Test] public void CreateScope_Resolve_ReadsParentAndCanOverrideWithoutPollutingParent() { @@ -152,6 +217,9 @@ namespace FlowScope.Tests.EditMode.Container Assert.Throws(() => container.RegisterInstance(new NamedService("late"))); Assert.Throws(() => container.RegisterFactory(_ => new NamedService("late"))); + Assert.Throws(() => container.RegisterSingletonFactory(_ => new NamedService("late"))); + Assert.Throws(() => container.RegisterTransient(_ => new NamedService("late"))); + Assert.Throws(() => container.RegisterScoped(_ => new NamedService("late"))); Assert.Throws(() => container.Resolve()); Assert.Throws(() => container.CreateScope()); } @@ -174,6 +242,24 @@ namespace FlowScope.Tests.EditMode.Container public void Dispose() => IsDisposed = true; } + private sealed class DisposableNamedService : IService, IDisposable + { + private readonly List _disposeOrder; + + public DisposableNamedService(string name, List disposeOrder) + { + Name = name; + _disposeOrder = disposeOrder; + } + + public string Name { get; } + + public void Dispose() + { + _disposeOrder.Add(Name); + } + } + private sealed class FirstDisposable : IDisposable { private readonly List _disposeOrder; diff --git a/My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs b/My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs index 8f3ac58..c23f405 100644 --- a/My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs +++ b/My project/Assets/FlowScope/Tests/EditMode/Flow/GameFlowTests.cs @@ -61,6 +61,25 @@ namespace FlowScope.Tests.EditMode.Flow Assert.That(SecondFeature.Instance.Context.Resources, Is.SameAs(services.Resource.Groups[1])); } + [Test] + public void SwitchToAsync_WhenReturningToFeatureType_CreatesNewFeatureInstance() + { + var services = new TestServices(); + var root = services.CreateRoot(); + var flow = new GameFlow(); + flow.StartupAsync(root, CancellationToken.None).GetAwaiter().GetResult(); + var firstEntry = FirstFeature.Instance; + + flow.SwitchToAsync(CancellationToken.None).GetAwaiter().GetResult(); + flow.SwitchToAsync(CancellationToken.None).GetAwaiter().GetResult(); + + Assert.That(FirstFeature.Instance, Is.Not.SameAs(firstEntry)); + Assert.That(firstEntry.Calls, Is.EqualTo(new[] { "Load", "Enter", "Exit", "Dispose" })); + Assert.That(FirstFeature.Instance.Calls, Is.EqualTo(new[] { "Load", "Enter" })); + Assert.That(services.Resource.Groups[0].IsDisposed, Is.True); + Assert.That(services.Resource.Groups[2].IsDisposed, Is.False); + } + [Test] public void StartupAsync_WhenFeatureLoadFails_CleansCreatedFeatureAndReturnsIdle() { @@ -114,7 +133,6 @@ namespace FlowScope.Tests.EditMode.Flow flow.StartupAsync(root, CancellationToken.None).GetAwaiter().GetResult(); LogAssert.Expect(LogType.Error, new Regex("System\\.InvalidOperationException: exit failed")); - LogAssert.Expect(LogType.Error, new Regex("System\\.InvalidOperationException: dispose failed")); LogAssert.Expect(LogType.Error, new Regex("System\\.AggregateException: One or more container-owned instances failed to dispose\\. \\(dispose failed\\)")); flow.ShutdownAsync(CancellationToken.None).GetAwaiter().GetResult(); @@ -168,11 +186,11 @@ namespace FlowScope.Tests.EditMode.Flow root.RegisterInstance(Config); root.RegisterInstance(Save); root.RegisterInstance(Resource); - root.RegisterFactory(_ => new FirstFeature()); - root.RegisterFactory(_ => new SecondFeature()); - root.RegisterFactory(_ => new FailingLoadFeature()); - root.RegisterFactory(_ => new FailingEnterFeature()); - root.RegisterFactory(_ => new ThrowingExitAndDisposeFeature()); + root.RegisterTransient(_ => new FirstFeature()); + root.RegisterTransient(_ => new SecondFeature()); + root.RegisterTransient(_ => new FailingLoadFeature()); + root.RegisterTransient(_ => new FailingEnterFeature()); + root.RegisterTransient(_ => new ThrowingExitAndDisposeFeature()); return root; } } diff --git a/My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs b/My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs index 034be1b..b115b00 100644 --- a/My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs +++ b/My project/Assets/FlowScope/Tests/EditMode/Resources/ResourceServiceBackendTests.cs @@ -52,6 +52,50 @@ namespace FlowScope.Tests.EditMode.Resources } } + [UnityTest] + public IEnumerator LoadAsync_WhenOneSharedWaiterCancels_OtherWaiterReceivesHandle() + { + var backend = new FakeResourceBackend(); + var service = new ResourceService(backend); + using var cts = new CancellationTokenSource(); + + var canceled = service.LoadAsync("shared", cts.Token); + var retained = service.LoadAsync("shared", CancellationToken.None); + cts.Cancel(); + backend.Gate.SetResult(new TestAsset()); + + yield return AwaitExpected(canceled); + yield return Await(retained); + + using var handle = Result(retained); + Assert.AreEqual(1, backend.LoadCalls); + Assert.IsNotNull(handle.Asset); + Assert.IsTrue(service.TryLoadCompleted("shared", out var cached)); + cached.Dispose(); + } + + [UnityTest] + public IEnumerator LoadAsync_WhenAllSharedWaitersCancel_ReleasesCompletedBackendAssetWithoutCaching() + { + var backend = new FakeResourceBackend(); + var service = new ResourceService(backend); + using var firstCts = new CancellationTokenSource(); + using var secondCts = new CancellationTokenSource(); + + var first = service.LoadAsync("shared", firstCts.Token); + var second = service.LoadAsync("shared", secondCts.Token); + firstCts.Cancel(); + secondCts.Cancel(); + backend.Gate.SetResult(new TestAsset()); + + yield return AwaitExpected(first); + yield return AwaitExpected(second); + + Assert.AreEqual(1, backend.LoadCalls); + Assert.AreEqual(1, backend.ReleaseCalls); + Assert.IsFalse(service.TryLoadCompleted("shared", out _)); + } + [UnityTest] public IEnumerator DisposeLastHandle_ReleasesBackendAsset() { diff --git a/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs b/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs index 1bb9539..0f2c94f 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/Samples/MainMenuP0Tests.cs @@ -49,6 +49,26 @@ namespace FlowScope.Tests.PlayMode.Samples LogAssert.NoUnexpectedReceived(); } + [UnityTest] + public IEnumerator Awake_StartsThroughObservableStartupTask() + { + DeleteSampleSave(); + var root = new GameObject("bootstrap-awake-test"); + var bootstrap = root.AddComponent(); + + Assert.That(bootstrap.StartupTask, Is.Not.Null); + yield return AwaitTask(bootstrap.StartupTask); + + var panel = Object.FindObjectOfType(true); + Assert.That(panel, Is.Not.Null); + Assert.That(panel.gameObject.activeSelf, Is.True); + + yield return AwaitTask(bootstrap.ShutdownAsync(CancellationToken.None)); + Object.DestroyImmediate(root); + DeleteSampleSave(); + LogAssert.NoUnexpectedReceived(); + } + [UnityTest] public IEnumerator ClickIncrement_UpdatesGoldTextImmediately() { diff --git a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs index dc0e519..e91d17b 100644 --- a/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs +++ b/My project/Assets/FlowScope/Tests/PlayMode/UI/UIPreloadServiceTests.cs @@ -1,6 +1,7 @@ using System.Collections; using System.Collections.Generic; using System.Threading; +using System.Threading.Tasks; using FlowScope.UI; using NUnit.Framework; using UnityEngine; @@ -58,6 +59,27 @@ namespace FlowScope.Tests.PlayMode.UI Assert.That(resources.Handles, Has.Count.EqualTo(1)); } + [UnityTest] + public IEnumerator PreloadAsync_WhenSamePanelLoadsConcurrently_UsesOneResourceLoad() + { + var prefab = CreatePrefab("ExplicitPathPrefab"); + var resources = new GatedUITestResources("Custom/Explicit", prefab); + var preload = new UIPreloadService(resources); + + var first = preload.PreloadAsync(CancellationToken.None); + var second = preload.PreloadAsync(CancellationToken.None); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + + resources.Complete(); + yield return UITestAsync.Await(first); + yield return UITestAsync.Await(second); + + Assert.That(resources.LoadCalls, Is.EqualTo(1)); + Assert.That(resources.Handle.IsDisposed, Is.False); + Assert.That(preload.IsPreloaded(), Is.True); + } + [UnityTest] public IEnumerator OpenAsync_WhenPanelIsPreloaded_ReusesPreloadedHandle() { @@ -111,5 +133,52 @@ namespace FlowScope.Tests.PlayMode.UI gameObject.AddComponent(); return gameObject; } + + private sealed class GatedUITestResources : FlowScope.Resources.IResourceService + { + private readonly string _key; + private readonly GameObject _asset; + private readonly TaskCompletionSource> _gate = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + public GatedUITestResources(string key, GameObject asset) + { + _key = key; + _asset = asset; + Handle = new UITestResourceHandle(key, asset); + } + + public int LoadCalls { get; private set; } + public UITestResourceHandle Handle { get; } + + public Task> LoadAsync( + string key, + CancellationToken cancellationToken) + where T : class + { + cancellationToken.ThrowIfCancellationRequested(); + LoadCalls++; + Assert.That(key, Is.EqualTo(_key)); + return AwaitHandle(); + } + + public FlowScope.Resources.IResourceGroup CreateGroup() + { + throw new System.NotSupportedException(); + } + + public void Complete() + { + _gate.SetResult(Handle); + } + + private async Task> AwaitHandle() + where T : class + { + var handle = await _gate.Task; + Assert.That(handle.Asset, Is.SameAs(_asset)); + return (FlowScope.Resources.IResourceHandle)(object)handle; + } + } } } diff --git a/docs/guides/flowscope-runtime-usage.md b/docs/guides/flowscope-runtime-usage.md index ceb29e5..539b6a8 100644 --- a/docs/guides/flowscope-runtime-usage.md +++ b/docs/guides/flowscope-runtime-usage.md @@ -2,15 +2,15 @@ ## Bootstrap -Bootstrap 负责组合运行时依赖，而不是把业务流程写进场景脚本。推荐顺序是创建根 `Container`、注册 Config/Resource/Save/UI/Audio 等服务、注册首个 Feature 的工厂，最后调用 `GameFlow.StartupAsync`。 +Bootstrap 负责组合运行时依赖，而不是把业务流程写进场景脚本。推荐顺序是创建根 `Container`、注册 Config/Resource/Save/UI/Audio 等服务、用 transient/scoped 生命周期注册首个 Feature，最后调用 `GameFlow.StartupAsync`。 -Bootstrap 必须持有生命周期 `CancellationToken`，退出时先保存必要状态，再调用 `GameFlow.ShutdownAsync`，最后释放预加载资源、音频句柄或其他根级缓存。 +Bootstrap 必须持有生命周期 `CancellationToken`，退出时先保存必要状态，再调用 `GameFlow.ShutdownAsync`，最后释放预加载资源、音频句柄或其他根级缓存。样例启动入口应暴露可观察的 `Task` 并统一记录异常，避免在 `Awake` / `OnApplicationQuit` 中直接使用 `async void`。 ## Container 注册规则 -容器注册应显式、可读、可追踪。稳定服务使用 `RegisterInstance`，带运行时参数或 Feature 私有依赖使用 `RegisterFactory`。不要依赖类型扫描或名称约定来猜测注册关系。 +容器注册应显式、可读、可追踪。稳定外部实例使用 `RegisterInstance`；根级懒加载单例使用 `RegisterSingletonFactory`；Feature、ViewModel、短生命周期服务使用 `RegisterTransient` 或 `RegisterScoped`。`RegisterFactory` 仅作为旧式 lazy singleton 兼容入口，不要用于 Feature 或临时业务对象。 -Feature 只从自己的 scope 解析依赖。跨 Feature 的共享服务放在 root container，Feature 内部临时对象放在 Feature scope，并随 `FeatureContext` 一起释放。 +Feature 只从自己的 scope 解析依赖。跨 Feature 的共享服务放在 root container，Feature 内部临时对象放在 Feature scope，并随 `FeatureContext` 一起释放。父容器中的 transient/scoped 注册被子 scope 解析时，由请求的子 scope 持有和释放；singleton 仍由注册所在 scope 持有。 ## Feature 生命周期 diff --git a/docs/requirements/p1-production-hardening.md b/docs/requirements/p1-production-hardening.md index 736f5a1..291024f 100644 --- a/docs/requirements/p1-production-hardening.md +++ b/docs/requirements/p1-production-hardening.md @@ -33,6 +33,6 @@ P1 在 P0 核心接口稳定后推进，目标是补齐第一批生产化扩展 ## 当前验收状态 -代码级编译验证中，`FlowScope.Runtime` 和 `FlowScope.Tests.EditMode` 可通过。`FlowScope.Tests.PlayMode` 在排除 Addressables 适配测试后可通过。 +代码级编译验证中，`FlowScope.Runtime`、`FlowScope.Tests.EditMode` 和 `FlowScope.Tests.PlayMode` 均可通过；仍存在 Unity 生成项目的引用版本冲突 warning。 -完整 PlayMode 与 Unity Test Runner 验收仍依赖 Unity 完成 `com.unity.addressables` 包解析；当前机器上主工程被其他 Unity 实例占用，且 `Library/PackageCache` 尚未出现 Addressables 包，因此本轮不能标记为完整人工验收通过。 +`packages-lock.json` 已包含 Addressables 相关包，旧的 PlayMode 编译阻塞已解除。完整 Unity Test Runner 与 MainMenuP0 人工场景验收仍需要单独补跑，不能仅用 `dotnet build` 替代。 diff --git a/docs/reviews/flowscope-framework-review.md b/docs/reviews/flowscope-framework-review.md index 865a94f..bdc764c 100644 --- a/docs/reviews/flowscope-framework-review.md +++ b/docs/reviews/flowscope-framework-review.md @@ -9,7 +9,7 @@ FlowScope 当前已经具备一个轻量游戏框架的基本形态，适合用于小型项目、原型项目或作为后续框架演进的 P0 基座。它的优势在于分层清晰、抽象克制、测试意识较好，并且围绕 Unity 常见横切能力建立了可替换接口。 -当前不建议直接定义为稳定生产框架。主要原因是资源异步、运行时对象销毁、Addressables 依赖方式和自研 JSON 转换器仍存在生产风险。这些问题不影响它作为框架雏形继续推进，但应作为进入生产复用前的前置整改项。 +当前不建议直接定义为稳定生产框架。主要原因是运行时对象销毁、自研 JSON 转换器、AOT/linker 验证和真实 Unity Test Runner 验收仍存在生产风险。这些问题不影响它作为框架雏形继续推进，但应作为进入生产复用前的前置整改项。 ## 2. 技术基线 @@ -57,7 +57,7 @@ FlowScope 当前已经具备一个轻量游戏框架的基本形态，适合用 代表文件：`Runtime/Container/Container.cs` -自研容器实现轻量，支持实例注册、工厂注册、作用域、父子容器、循环依赖检测和拥有对象释放。 +自研容器实现轻量，支持实例注册、singleton factory、scoped、transient、父子容器、循环依赖检测和拥有对象释放。 优点： @@ -70,30 +70,29 @@ FlowScope 当前已经具备一个轻量游戏框架的基本形态，适合用 - `GeneratedFactories` 需要外部注册，长期需要配套生成器或明确手写规范。 - 反射工厂适合开发期，但移动端/AOT 环境需要持续验证。 -- 当前没有生命周期枚举，例如 transient、scoped、singleton，实际行为需要文档明确。 +- `RegisterFactory` 仍保留为旧式 lazy singleton 兼容入口，需要避免在新代码中用于 Feature/ViewModel/短生命周期对象。 -评价：P0 阶段可接受。后续重点不是扩大能力，而是写清注册规范和 AOT 策略。 +评价：P0/P1 阶段可接受。后续重点不是扩大能力，而是保持生命周期语义稳定、写清注册规范和 AOT 策略。 ### 3.3 Resources -代表文件：`Runtime/Resources/AddressablesResourceService.cs` +代表文件：`Runtime/Resources/ResourceService.cs`、`Addressables/AddressablesResourceBackend.cs` -资源层抽象了 `IResourceService`、`IResourceHandle` 和 `IResourceGroup`，并实现了 Addressables 加载、引用计数和分组释放。 +资源层抽象了 `IResourceService`、`IResourceHandle`、`IResourceGroup` 和 `IResourceBackend`，Runtime 核心负责共享加载、引用计数和分组释放，Addressables 适配位于独立 asmdef。 优点： - 使用句柄释放资源，方向正确。 - 支持同 key 同类型加载去重。 - `ResourceGroup` 适合跟随 Feature 生命周期批量释放。 -- PlayMode 测试覆盖了复用、引用计数和取消等路径。 +- EditMode / PlayMode 测试覆盖了复用、引用计数、取消等待和完成后释放等路径。 风险： -- Addressables 通过字符串反射调用，编译期无法发现 API 变化。 -- Runtime asmdef 没有显式引用 Addressables，当前设计牺牲了类型安全。 -- 取消等待后底层 Addressables 任务可能仍继续执行，后续需要确认资源完成后的释放策略。 +- Addressables 真实 load/release 仍缺少最小集成测试资源。 +- 取消等待后底层共享加载仍会继续执行，这是设计选择；需要继续用测试保护“所有等待者取消后完成即释放”的语义。 -评价：抽象方向正确，但 Addressables 适配层应从 Runtime 核心拆出，改为显式依赖的独立 asmdef。 +评价：抽象方向正确，Runtime 核心与 Addressables 适配层已经拆开；下一步重点是真实 Addressables 集成验收。 ### 3.4 UI @@ -130,11 +129,11 @@ UI 模块基于 `UIPanelAttribute` 标记层级和路径，通过 `UIManager` 风险： -- `LoadClip` 内部使用 `.GetAwaiter().GetResult()` 同步等待异步资源加载，在 Unity 主线程和 Addressables 场景下存在卡顿或死锁风险。 +- 同步播放 API 只适合读取已经预加载或已完成的资源；调用者如果误用未预加载资源，会得到明确异常而不是阻塞等待。 - 音频服务是 `MonoBehaviour`，但依赖通过 `Initialize` 注入，需要文档明确创建流程。 -- 当前播放接口是同步方法，与异步资源系统存在模型冲突。 +- 新业务仍应优先使用 `PlayBgmAsync` / `PlaySfxAsync`，避免把同步 API 当作资源加载入口。 -评价：功能够用，但异步资源加载方式必须调整后才适合生产复用。 +评价：功能够用，旧的同步阻塞风险已降低；生产复用前仍需继续明确同步 API 的使用边界。 ### 3.6 Save / Config / Data @@ -183,12 +182,11 @@ UI 模块基于 `UIPanelAttribute` 标记层级和路径，通过 `UIManager` | 优先级 | 风险 | 影响 | 建议 | | --- | --- | --- | --- | -| P0 | `AudioService` 同步等待异步资源 | 可能导致主线程卡顿或死锁 | 改为异步播放或预加载机制 | | P0 | Runtime 使用 `DestroyImmediate` | 运行时生命周期不稳 | 改为 `Destroy`，测试按帧等待 | -| P1 | Addressables 反射调用 | 类型安全弱，升级风险高 | 拆独立 Addressables 适配 asmdef | +| P1 | Addressables 真实集成测试不足 | 适配层编译通过不等于真实 load/release 验收通过 | 增加最小 Addressables 测试资源或人工验收步骤 | | P1 | 自研 JSON 转换器 | 边界和维护成本高 | 明确 DTO 限制或接入成熟 JSON 库 | | P1 | `GameFlow` 固定加载配置 | 启动流程不够弹性 | 配置加载移到 Bootstrap 或可选策略 | -| P2 | 容器生命周期语义文档不足 | 使用者容易误判对象生命周期 | 补充注册和释放规范 | +| P2 | `RegisterFactory` 兼容入口仍可能误用 | 使用者可能把 lazy singleton 用于短生命周期对象 | 新代码使用 `RegisterSingletonFactory` / `RegisterScoped` / `RegisterTransient` | ## 6. 审批意见 @@ -209,9 +207,9 @@ UI 模块基于 `UIPanelAttribute` 标记层级和路径，通过 `UIManager` 前置整改项： 1. 将 `UIManager` 运行时销毁逻辑从 `DestroyImmediate` 调整为 `Destroy`。 -2. 将 `AudioService` 的资源加载从同步阻塞改为异步或预加载模型。 -3. 为 Addressables 适配明确依赖策略，优先拆出独立 asmdef。 -4. 补充框架使用约定文档，包括 Bootstrap、Feature、资源释放、UI 路径、配置 DTO 限制。 +2. 为 Addressables 适配增加真实 load/release 最小验收。 +3. 明确 Config/Save DTO 与 AOT/linker 限制。 +4. 在进入 P2 包分发前补一次 Unity Test Runner 全量验收记录。 ## 7. 后续建议 @@ -239,23 +237,23 @@ UI 模块基于 `UIPanelAttribute` 标记层级和路径，通过 `UIManager` | --- | --- | --- | | 架构清晰度 | 良好 | 模块边界清楚，抽象克制 | | 可测试性 | 良好 | 已有较完整 EditMode/PlayMode 测试 | -| Unity 生命周期安全 | 一般 | `DestroyImmediate` 和同步等待需修正 | +| Unity 生命周期安全 | 一般 | `DestroyImmediate` 与 Unity Test Runner 验收仍需收口 | | 生产稳定性 | 一般 | 仍处于 P0/P1 框架雏形 | | 可演进性 | 良好 | 适合继续拆模块、补规范、稳定 API | 综合评级：B+。 -结论摘要：FlowScope 是一个有工程意识的轻量 Unity 框架骨架，值得继续推进；但在正式生产复用前，需要优先修正异步资源、运行时销毁和 Addressables 依赖方式三个关键问题。 +结论摘要：FlowScope 是一个有工程意识的轻量 Unity 框架骨架，值得继续推进；但在正式生产复用前，需要优先修正运行时销毁、真实 Addressables 验收和 AOT/JSON 边界三个关键问题。 # P1 更新记录（2026-05-20） P1 已完成第一批生产化硬化，评审风险状态同步如下： | 原风险 | P1 状态 | 说明 | | --- | --- | --- | -| Addressables 反射调用 | 已降低 | Addressables 适配已从 Runtime 核心拆出到 `FlowScope.Addressables` 独立 asmdef，核心资源系统通过 `IResourceBackend` 工作。最终验收仍等待 Unity 完成 Addressables 包解析。 | +| Addressables 反射调用 | 已降低 | Addressables 适配已从 Runtime 核心拆出到 `FlowScope.Addressables` 独立 asmdef，核心资源系统通过 `IResourceBackend` 工作。包解析与生成项目编译阻塞已解除，仍缺真实 load/release 验收。 | | 自研 JSON 转换器缺少扩展点 | 已降低 | Config 增加 `IConfigSource` / `IConfigParser`，Save 增加版本 envelope 与 `ISaveMigration` 链。长期是否替换成熟 JSON 库仍是 P2 决策。 | | UI 缺少轻量导航与预加载 | 已降低 | 新增 `UIScreenNavigator` 与 `UIPreloadService`，并补充 PlayMode 编译级测试。 | | 样例不能体现生产化装配 | 已降低 | MainMenuP0 已改为 P1 装配方式，覆盖启动、点击、保存恢复和资源释放。 | | Package/editor tooling | 保持 P2 | 本轮没有做包分发、编辑器工具和跨项目模板化。 | -当前结论仍是“有条件通过”：P1 代码与文档已落地，但完整 Unity Test Runner 和人工场景验收需要在 Unity 工程未被占用、Addressables 包解析完成后补跑。 +当前结论仍是“有条件通过”：P1 代码与文档已落地，P2 前整改已收束 Container/Feature 生命周期、样例 Bootstrap 启动模型、资源取消与 UI 预加载边界；完整 Unity Test Runner 和人工场景验收仍需补跑。 diff --git a/docs/reviews/p1-architecture-audit-before-p2.md b/docs/reviews/p1-architecture-audit-before-p2.md new file mode 100644 index 0000000..6b161fa --- /dev/null +++ b/docs/reviews/p1-architecture-audit-before-p2.md @@ -0,0 +1,423 @@ +# 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` 创建 feature scope 后调用 `scope.Resolve()`。 +- `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 scope，Feature 临时对象随 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(Func 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` 没有同类型并发去重。 +- `UIManager` 对预加载 handle 使用空释放句柄，实际释放依赖 `UIPreloadService.Release` / `ReleaseAll`。 + +影响： + +- 两个并发预加载可能重复加载并覆盖 handle。 +- UI 实例关闭不释放预加载 handle 是合理设计，但需要更强文档和测试防误用。 + +整改建议： + +- 为 `UIPreloadService` 增加同 Panel 类型的 in-flight load 合并。 +- 增加测试：并发 Preload 只触发一次底层加载。 +- 增加测试：已打开预加载 Panel 时提前 Release 的行为被定义清楚。 + +--- + +### 6. `UIScreenNavigator` 是轻量栈，不应被视为完整路由 + +风险等级：中 + +证据： + +- `UIScreenNavigator` 只保存 `Action` close stack。 +- 外部 `UIManager.Close` / `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` 成功加载、取消、失败、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. Container：singleton / 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 生命周期问题。 +- 不要把样例变成特殊路径；样例应继续代表推荐使用方式。 diff --git a/docs/reviews/p1-completion-report.md b/docs/reviews/p1-completion-report.md index f33c782..d12d18e 100644 --- a/docs/reviews/p1-completion-report.md +++ b/docs/reviews/p1-completion-report.md @@ -18,16 +18,15 @@ - `dotnet build "My project\FlowScope.Runtime.csproj" --no-restore`：通过，0 错误；存在 Unity 生成项目的引用版本冲突警告。 - `dotnet build "My project\FlowScope.Tests.EditMode.csproj" --no-restore`：通过，0 错误；存在相同引用警告。 -- `dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore`：完整编译失败，原因是 `FlowScope.Addressables` 生成项目和 `com.unity.addressables` 包尚未由 Unity 解析。 -- 临时排除 `AddressablesResourceBackendTests.cs` 后的 PlayMode 编译：通过，0 错误；用于确认 UI 与样例集成未破坏。 +- `dotnet build "My project\FlowScope.Tests.PlayMode.csproj" --no-restore`：通过，0 错误；存在相同引用警告。旧的 Addressables 包解析阻塞已解除。 +- 2026-05-21 P2 前整改补充：Container 已区分 `RegisterSingletonFactory` / `RegisterScoped` / `RegisterTransient`，Feature 示例改为 transient 注册，ResourceService 与 UIPreloadService 增加共享加载/并发预加载边界测试。 ## 人工验收 -尚未完成。主工程当前被其他 Unity 实例占用，Unity batchmode 无法打开同一 project path，无法执行 Test Runner 全量 PlayMode 和 MainMenuP0 场景人工验收。 +代码级编译验收已恢复。Unity Test Runner 和 MainMenuP0 场景人工验收仍需在 Unity 工程可用时补跑，不能仅用 `dotnet build` 替代。 -待 Unity 可用后，需要重新执行： +需要重新执行： -- Unity 包解析，确认 `Packages/packages-lock.json` 更新并出现 `com.unity.addressables`。 - PlayMode: `FlowScope.Tests.PlayMode.Samples.MainMenuP0Tests`。 - PlayMode 全量测试。 - 打开 `My project/Assets/FlowScope/Samples/MainMenuP0/Scenes/MainMenuP0.unity` 做人工点击、保存恢复、资源释放验收。 @@ -43,8 +42,8 @@ ## P2 建议 -- 让 Unity 完成 Addressables 包解析后补一次 P1 验收提交，包含 `packages-lock.json` 和 Unity Test Runner 结果。 -- 将 AudioService 的同步资源加载改为异步或预加载模型。 +- 补一次 Unity Test Runner 验收记录，确认 EditMode / PlayMode 在 Unity 内全绿。 +- 继续跟踪 AudioService 同步 API 的使用边界：同步方法只适合读取已预加载/已完成资源，新增业务应优先使用 async API。 - 为 Runtime/package 分发建立 asmdef、Samples、Tests 的包结构规范。 - 为 Config/Save 引入更成熟 JSON 库或明确长期 DTO 限制。 - 增加 AOT/linker 验证，避免反射和 JSON 构造在移动端出现隐性问题。