From 1dd5706e156c8563434de523e31a463308ef0f99 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 14 Aug 2026 21:14:08 +0200 Subject: [PATCH] Make character UI retries transactional --- .../2026-08-14-la8-character-management-ui.md | 7 +- .../Layout/CharacterManagementUiController.cs | 105 ++++++++++++----- .../CharacterManagementUiMountCoordinator.cs | 35 ++++-- .../UI/Layout/RetailDialogFactory.cs | 67 +++++++++-- .../CharacterManagementUiControllerTests.cs | 71 ++++++++++++ .../UI/Layout/RetailDialogFactoryTests.cs | 109 ++++++++++++++++++ 6 files changed, 352 insertions(+), 42 deletions(-) diff --git a/docs/research/2026-08-14-la8-character-management-ui.md b/docs/research/2026-08-14-la8-character-management-ui.md index 1bde915c..a769c5e0 100644 --- a/docs/research/2026-08-14-la8-character-management-ui.md +++ b/docs/research/2026-08-14-la8-character-management-ui.md @@ -104,7 +104,12 @@ retries on the next frame. Initial dialog-catalog, character root, and string misses likewise retry on later ticks without mounting a duplicate root or controller. Dialog presenter/catalog failures move their contexts to an internal retry ledger, so UI callbacks do not retain poisoned active/queued -entries and the same context can appear after resource recovery. +entries and the same context can appear after resource recovery. Priority +contexts remain ahead of ordinary retries and preserve retail's nested +preemption order when creation recovers. The mount coordinator owns a detached +controller before attaching its root or running the first template-resolving +tick; any partial failure disposes that exact controller before retry, so roots +and handlers cannot accumulate. There is deliberately no 3D preview and no claimed character-select background scene. The screen root remains neutral with respect to render-loop background diff --git a/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs b/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs index 02f27ce5..cd5aa341 100644 --- a/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs +++ b/src/AcDream.App/UI/Layout/CharacterManagementUiController.cs @@ -116,6 +116,36 @@ internal sealed class CharacterManagementUiController : IDisposable RetailDialogFactory dialogs, CharacterSelectionRuntimeBindings bindings, DialogStrings strings) + { + CharacterManagementUiController? controller = CreateDetached( + host, + layout, + templateResolver, + dialogs, + bindings, + strings); + if (controller is null) + return null; + + try + { + controller.AttachAndTick(); + return controller; + } + catch + { + controller.Dispose(); + throw; + } + } + + internal static CharacterManagementUiController? CreateDetached( + UiRoot host, + ImportedLayout layout, + Func templateResolver, + RetailDialogFactory dialogs, + CharacterSelectionRuntimeBindings bindings, + DialogStrings strings) { ArgumentNullException.ThrowIfNull(host); ArgumentNullException.ThrowIfNull(layout); @@ -144,20 +174,37 @@ internal sealed class CharacterManagementUiController : IDisposable } list.TemplateResolver = templateResolver; - var controller = new CharacterManagementUiController( - host, - layout, - list, - create, - enter, - delete, - restore, - dialogs, - bindings, - strings); - host.AddChild(controller.Root); - controller.Tick(); - return controller; + try + { + return new CharacterManagementUiController( + host, + layout, + list, + create, + enter, + delete, + restore, + dialogs, + bindings, + strings); + } + catch + { + list.TemplateResolver = null; + create.OnClick = null; + enter.OnClick = null; + delete.OnClick = null; + restore.OnClick = null; + throw; + } + } + + internal void AttachAndTick() + { + ObjectDisposedException.ThrowIf(_disposed, this); + if (Root.Parent is null) + _host.AddChild(Root); + Tick(); } private static bool ContainsViewport(UiElement element) @@ -247,20 +294,26 @@ internal sealed class CharacterManagementUiController : IDisposable if (_disposed) return; _disposed = true; - CloseAllDialogs(suppressCallbacks: true); - _enter.OnClick = null; - _delete.OnClick = null; - _restore.OnClick = null; - foreach (UiButton row in _rows) + try { - row.OnClick = null; - row.OnDoubleClick = null; + CloseAllDialogs(suppressCallbacks: true); + } + finally + { + _enter.OnClick = null; + _delete.OnClick = null; + _restore.OnClick = null; + foreach (UiButton row in _rows) + { + row.OnClick = null; + row.OnDoubleClick = null; + } + _rows.Clear(); + _rowIds.Clear(); + _list.Flush(); + _list.TemplateResolver = null; + _host.RemoveChild(Root); } - _rows.Clear(); - _rowIds.Clear(); - _list.Flush(); - _list.TemplateResolver = null; - _host.RemoveChild(Root); } private static bool TryCaptureRoster( diff --git a/src/AcDream.App/UI/Layout/CharacterManagementUiMountCoordinator.cs b/src/AcDream.App/UI/Layout/CharacterManagementUiMountCoordinator.cs index e297d5ac..e2748193 100644 --- a/src/AcDream.App/UI/Layout/CharacterManagementUiMountCoordinator.cs +++ b/src/AcDream.App/UI/Layout/CharacterManagementUiMountCoordinator.cs @@ -53,23 +53,42 @@ internal sealed class CharacterManagementUiMountCoordinator : IDisposable if (resources is null) return; - Controller = CharacterManagementUiController.Bind( + CharacterManagementUiController? candidate = + CharacterManagementUiController.CreateDetached( _host, resources.Layout, resources.TemplateResolver, dialogs, _bindings, resources.Strings); - if (Controller is not null) - { - Console.WriteLine( - $"[UI] retail character management from enum table 5 " - + $"(0x10000005 -> 0x{resources.LayoutId:X8}, " - + "root 0x1000039A; flat list, no viewport)."); - } + if (candidate is null) + return; + + // Take ownership before the first attach/tick. Template resolution + // happens inside that tick and can throw after the root and button + // handlers are live; the catch below can therefore always retire + // the exact partial controller before a later retry. + Controller = candidate; + candidate.AttachAndTick(); + Console.WriteLine( + $"[UI] retail character management from enum table 5 " + + $"(0x10000005 -> 0x{resources.LayoutId:X8}, " + + "root 0x1000039A; flat list, no viewport)."); } catch (Exception error) { + CharacterManagementUiController? partial = Controller; + Controller = null; + try + { + partial?.Dispose(); + } + catch (Exception cleanupError) + { + Console.WriteLine( + "[UI] character management partial-mount cleanup failed: " + + cleanupError.Message); + } Console.WriteLine( "[UI] character management mount will retry after resource " + $"recovery: {error.Message}"); diff --git a/src/AcDream.App/UI/Layout/RetailDialogFactory.cs b/src/AcDream.App/UI/Layout/RetailDialogFactory.cs index f631adb1..f72d7283 100644 --- a/src/AcDream.App/UI/Layout/RetailDialogFactory.cs +++ b/src/AcDream.App/UI/Layout/RetailDialogFactory.cs @@ -15,6 +15,7 @@ public sealed class RetailDialogFactory : IDisposable public required RetailDialogData Data { get; init; } public required uint Context { get; init; } public required uint QueueKey { get; init; } + public required ulong Sequence { get; init; } public Action? Callback { get; init; } public IRetailDialogView? View { get; set; } } @@ -27,6 +28,7 @@ public sealed class RetailDialogFactory : IDisposable private readonly LinkedList _retryable = new(); private readonly List _openOrder = new(); private uint _globalContext; + private ulong _globalSequence; private bool _resetting; private bool _disposed; @@ -90,6 +92,7 @@ public sealed class RetailDialogFactory : IDisposable Data = ownedData, Context = context, QueueKey = queueKey, + Sequence = NextSequence(), Callback = callback, }; @@ -106,7 +109,7 @@ public sealed class RetailDialogFactory : IDisposable if (!_activeQueued.TryGetValue(queueKey, out DialogInfo? current)) { - if (HasRetry(queueKey)) + if (HasRetry(queueKey) && !IsPriority(info)) { PendingQueue(queueKey).AddLast(info); return context; @@ -122,7 +125,7 @@ public sealed class RetailDialogFactory : IDisposable } LinkedList queue = PendingQueue(queueKey); - if (!ownedData.GetBoolean(RetailDialogProperty.Priority)) + if (!IsPriority(info)) { queue.AddLast(info); UpdatePendingDialogDisplays(); @@ -324,6 +327,14 @@ public sealed class RetailDialogFactory : IDisposable return _globalContext; } + private ulong NextSequence() + { + _globalSequence++; + if (_globalSequence == 0uL) + _globalSequence++; + return _globalSequence; + } + private LinkedList PendingQueue(uint queueKey) { if (_pending.TryGetValue(queueKey, out LinkedList? queue)) @@ -480,14 +491,37 @@ public sealed class RetailDialogFactory : IDisposable continue; } - if (!_activeQueued.ContainsKey(info.QueueKey) - && ReferenceEquals(FirstRetry(info.QueueKey), info)) - { + if (!ReferenceEquals(FirstRetry(info.QueueKey), info)) + continue; + + if (!_activeQueued.TryGetValue( + info.QueueKey, + out DialogInfo? active)) TryActivateRetry(info.QueueKey); - } + else if (IsPriority(info) + && (!IsPriority(active) || info.Sequence > active.Sequence)) + TryPreemptWithRetry(info, active); } } + private void TryPreemptWithRetry(DialogInfo priority, DialogInfo current) + { + LinkedList queue = PendingQueue(priority.QueueKey); + Suspend(current); + queue.AddFirst(current); + _activeQueued[priority.QueueKey] = priority; + _retryable.Remove(priority); + if (TryCreateDialog(priority)) + return; + + _activeQueued.Remove(priority.QueueKey); + queue.Remove(current); + if (queue.Count == 0) + _pending.Remove(priority.QueueKey); + OpenSpecificDialog(current); + QueueRetry(priority); + } + private bool TryActivateRetry(uint queueKey) { DialogInfo? info = FirstRetry(queueKey); @@ -512,10 +546,29 @@ public sealed class RetailDialogFactory : IDisposable private bool HasRetry(uint queueKey) => FirstRetry(queueKey) is not null; + private static bool IsPriority(DialogInfo info) => + info.Data.GetBoolean(RetailDialogProperty.Priority); + private void QueueRetry(DialogInfo info) { - if (!_retryable.Contains(info)) + if (_retryable.Contains(info)) + return; + if (!IsPriority(info)) + { _retryable.AddLast(info); + return; + } + + LinkedListNode? existing = _retryable.First; + while (existing is not null + && existing.Value.QueueKey != info.QueueKey) + { + existing = existing.Next; + } + if (existing is null) + _retryable.AddLast(info); + else + _retryable.AddBefore(existing, info); } private void UpdatePendingDialogDisplays() diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterManagementUiControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterManagementUiControllerTests.cs index 9cc8a664..d9708e38 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterManagementUiControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterManagementUiControllerTests.cs @@ -188,6 +188,66 @@ public sealed class CharacterManagementUiControllerTests Assert.Equal(3, resourceAttempts); } + [Fact] + public void MountCoordinator_PostAttachFailuresDisposeBeforeRetryAndRecovery() + { + var host = new UiRoot { Width = 800f, Height = 600f }; + var runtime = new FakeRuntime(); + using var dialogs = new RetailDialogFactory( + host, + RetailDialogFactoryTests.BuildDialogLayout); + var screens = new List(); + int failingAttempts = 2; + using var coordinator = new CharacterManagementUiMountCoordinator( + host, + runtime.Bindings, + () => dialogs, + () => + { + ImportedLayout screen = BuildScreen(); + screens.Add(screen); + bool throwAfterAttach = failingAttempts-- > 0; + return new CharacterManagementUiMountResources( + 0x21000004u, + screen, + (_, _) => throwAfterAttach + ? throw new InvalidOperationException( + "template failed after root attach") + : BuildRow(), + TestStrings()); + }); + + coordinator.Tick(); + Assert.Null(coordinator.Controller); + Assert.Empty(host.Children); + Assert.Single(screens); + AssertDetachedAndUnbound(screens[0]); + + coordinator.Tick(); + Assert.Null(coordinator.Controller); + Assert.Empty(host.Children); + Assert.Equal(2, screens.Count); + Assert.All(screens, AssertDetachedAndUnbound); + + coordinator.Tick(); + CharacterManagementUiController mounted = Assert.IsType< + CharacterManagementUiController>(coordinator.Controller); + Assert.Equal(3, screens.Count); + Assert.Single(host.Children); + Assert.Same(mounted.Root, host.Children[0]); + Assert.NotNull(Assert.IsType(screens[2].FindElement( + CharacterManagementUiController.EnterElementId)).OnClick); + + coordinator.Tick(); + Assert.Equal(3, screens.Count); + Assert.Single(host.Children); + + coordinator.Dispose(); + Assert.Empty(host.Children); + Assert.Null(coordinator.Controller); + Assert.All(screens, AssertDetachedAndUnbound); + } + [Fact] public void DeleteConfirmation_IsCaseInsensitive_ThenWaitsThroughAckUntilFreshRoster() { @@ -509,6 +569,17 @@ public sealed class CharacterManagementUiControllerTests "Please Wait", "Entering World"); + private static void AssertDetachedAndUnbound(ImportedLayout screen) + { + Assert.Null(screen.Root.Parent); + Assert.Null(Assert.IsType(screen.FindElement( + CharacterManagementUiController.EnterElementId)).OnClick); + Assert.Null(Assert.IsType(screen.FindElement( + CharacterManagementUiController.DeleteElementId)).OnClick); + Assert.Null(Assert.IsType(screen.FindElement( + CharacterManagementUiController.RestoreElementId)).OnClick); + } + private static ImportedLayout BuildScreen(bool includePreview = false) { var root = new ElementInfo diff --git a/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs b/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs index 76510dd9..8baca976 100644 --- a/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs @@ -542,6 +542,110 @@ public sealed class RetailDialogFactoryTests Assert.True(factory.CloseDialog(later)); } + [Fact] + public void PriorityRequest_PreemptsOrdinaryRetryAndPreservesOrdinaryFifo() + { + var root = new UiRoot { Width = 800f, Height = 600f }; + bool waitsAvailable = false; + using var factory = new RetailDialogFactory(root, type => + type == RetailDialogType.Wait && !waitsAvailable + ? null + : BuildDialogLayout(type)); + + uint failed = factory.MakeWait("failed ordinary"); + uint later = factory.MakeWait("later ordinary"); + uint priority = factory.MakeDialog( + Priority(RetailDialogData.Message("priority"))); + + Assert.Equal(1, factory.ActiveCount); + Assert.Equal(1, factory.RetryCount); + Assert.Equal(1, factory.PendingCount); + Assert.Equal("priority", MessageFromAnyDialog(root.Modal!)); + + Assert.True(factory.CloseDialog(priority)); + Assert.Equal(0, factory.ActiveCount); + Assert.Equal(1, factory.RetryCount); + Assert.Equal(1, factory.PendingCount); + + waitsAvailable = true; + factory.Tick(); + Assert.Equal("failed ordinary", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(failed)); + Assert.Equal("later ordinary", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(later)); + } + + [Fact] + public void FailedPriority_RetriesAheadOfRestoredActiveAndQueuedDialog() + { + var root = new UiRoot { Width = 800f, Height = 600f }; + bool priorityAvailable = false; + using var factory = new RetailDialogFactory(root, type => + type == RetailDialogType.Message && !priorityAvailable + ? null + : BuildDialogLayout(type)); + + uint active = factory.MakeWait("active"); + uint queued = factory.MakeWait("queued"); + uint priority = factory.MakeDialog( + Priority(RetailDialogData.Message("priority"))); + + Assert.Equal(1, factory.ActiveCount); + Assert.Equal(1, factory.RetryCount); + Assert.Equal(1, factory.PendingCount); + Assert.Equal("active", MessageFromAnyDialog(root.Modal!)); + + priorityAvailable = true; + factory.Tick(); + + Assert.Equal(1, factory.ActiveCount); + Assert.Equal(0, factory.RetryCount); + Assert.Equal(2, factory.PendingCount); + Assert.Equal("priority", MessageFromAnyDialog(root.Modal!)); + + Assert.True(factory.CloseDialog(priority)); + Assert.Equal("active", MessageFromAnyDialog(root.Modal!)); + Assert.Equal(1, factory.PendingCount); + Assert.True(factory.CloseDialog(active)); + Assert.Equal("queued", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(queued)); + } + + [Fact] + public void MultipleRetries_NewestPriorityFirstThenOlderPriorityThenOrdinaryFifo() + { + var root = new UiRoot { Width = 800f, Height = 600f }; + bool available = false; + using var factory = new RetailDialogFactory(root, type => + !available ? null : BuildDialogLayout(type)); + + uint ordinary = factory.MakeWait("ordinary"); + uint later = factory.MakeWait("later"); + uint olderPriority = factory.MakeDialog( + Priority(RetailDialogData.Message("older priority"))); + uint newerPriority = factory.MakeDialog(Priority( + RetailDialogData.ConfirmationTextInput("newer priority"))); + + Assert.Equal(0, factory.ActiveCount); + Assert.Equal(3, factory.RetryCount); + Assert.Equal(1, factory.PendingCount); + + available = true; + factory.Tick(); + + Assert.Equal(1, factory.ActiveCount); + Assert.Equal(2, factory.RetryCount); + Assert.Equal("newer priority", MessageFromAnyDialog(root.Modal!)); + + Assert.True(factory.CloseDialog(newerPriority)); + Assert.Equal("older priority", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(olderPriority)); + Assert.Equal("ordinary", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(ordinary)); + Assert.Equal("later", MessageFromAnyDialog(root.Modal!)); + Assert.True(factory.CloseDialog(later)); + } + private static RetailDialogFactory CreateFactory( UiRoot root, List layouts) @@ -575,6 +679,11 @@ public sealed class RetailDialogFactoryTests .LinesProvider() .Select(static line => line.Text)); + private static RetailDialogData Priority(RetailDialogData data) => + data.Set(RetailDialogProperty.Priority, true) + .Set(RetailDialogProperty.QueueKey, + RetailDialogFactory.DefaultQueueKey); + internal static ImportedLayout BuildDialogLayout(RetailDialogType type) { uint rootId = RetailDialogFactory.RootElementId(type);